lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Effects UI rewrite

Open enp2s0 opened this issue 1 year ago • 36 comments

Recent commits introduced some merge conflicts with #7242; this PR replaces that one. This PR also does not attempt to make the EffectView resizable or modify the wet/dry behavior and instead just focuses on UI improvements:

  • Hide the auto quit decay knob if autoquit is disabled in settings.
  • Remove the gate knob.
  • Shrink the height of effects from 60px to 35px.
  • Use the effect label itself as the button to open the controls rather than a separate button.

The new design in action: image

And in the classic theme (as an aside, the new mixer strip design doesn't seem to be compatible at all with the classic theme, but the FX rack is good enough): image

~This is mostly complete. Currently hunting down a small bug where the controls button stays in the "open" state even when you close the FX window, but this is likely an easy fix.~ Issue is fixed.

Replaces #7242.

enp2s0 avatar Aug 07 '24 16:08 enp2s0

I said it there and I'll say it again, consider spinning up the gate knob removal into it's own seperate PR. Makes it easier to review.

Rossmaxx avatar Aug 08 '24 02:08 Rossmaxx

@enp2s0, I took the liberty to apply some changes (forward includes, newline at end of file) myself. That's faster than some back and forth in comment. I hope that's okay for you.

michaelgregorius avatar Aug 09 '24 18:08 michaelgregorius

@michaelgregorius all good, thanks!

enp2s0 avatar Aug 09 '24 19:08 enp2s0

Can we preserve the knob labels? I think it would be clearer that way. Also, how about making the effect buttons more consistent in style to the track buttons (i.e. they're flat and borderless when not pressed)?

DomClark avatar Aug 09 '24 21:08 DomClark

@DomClark As far as knob labels are concerned the reason for removing them is to significantly reduce the required height. I did try adding labels to the left or right at one point, but it leaves very little space for the effect name/button itself, and increasing the width of the effect chain widget causes it to not fit properly inside the 400x400 windows for instruments.

As far as making the FX buttons match the track buttons, this is a good idea. Working on implementing that now.

enp2s0 avatar Aug 11 '24 01:08 enp2s0

I personally am willing to trade a little bit of size reduction for clear knob labels

Rossmaxx avatar Aug 11 '24 03:08 Rossmaxx

Updated screenshots?

Rossmaxx avatar Aug 12 '24 00:08 Rossmaxx

Knob labels are back. I honestly prefer the look without them but it seems I am in the minority on this. image

enp2s0 avatar Aug 12 '24 01:08 enp2s0

Great. Sounds like a nitpick at this point, but can you vertically make the effect button larger? That way, it'll look slightly better.

Rossmaxx avatar Aug 12 '24 01:08 Rossmaxx

image It looks better height-wise but the knobs don't look centered with the labels -- any idea why that is? I'd like to avoid just ->move()ing them since the rest is done with layouts.

enp2s0 avatar Aug 12 '24 01:08 enp2s0

any idea why that is?

Consider wrapping the knob + label on a QVBoxLayout and see if it fixes it.

Rossmaxx avatar Aug 12 '24 01:08 Rossmaxx

The label and the knob are already wrapped together -- I'm just running setLabel on the knob directly. It almost seems like the label and the knob have different margin settings (without the label, the knob was centered).

enp2s0 avatar Aug 12 '24 01:08 enp2s0

Great. Sounds like a nitpick at this point, but can you vertically make the effect button larger? That way, it'll look slightly better.

I second this request. IMO the button should be almost as big as the surrounding effect box so that it is easy to hit.

The text in the buttons also looks very cramped. I think some larger margins would be good here.

michaelgregorius avatar Aug 12 '24 15:08 michaelgregorius

Knob labels are back. I honestly prefer the look without them but it seems I am in the minority on this. image

I'm actually in favour of removing label too, as there's tooltips. The issue is that tooltips aren't enough and those knobs should just be there for convenience and should be mirrored in a bar on top of the FX windows (which would have labels, just like it's done for LMMS instruments Volume and Panning knobs), which, as of now, doesn't exist yet. So it's probably better to keep the for now, although it clearly reduces in effectiveness the compactness the PR was trying to achieve.

RoxasKH avatar Aug 12 '24 16:08 RoxasKH

Great. Sounds like a nitpick at this point, but can you vertically make the effect button larger? That way, it'll look slightly better.

I second this request. IMO the button should be almost as big as the surrounding effect box so that it is easy to hit. The text in the buttons also looks very cramped. I think some larger margins would be good here.

This is left and we good for merge.

Rossmaxx avatar Aug 20 '24 06:08 Rossmaxx

I tried this, but the only problem is the "W/D" label next to the knob, because visually it makes the plugin space look unnecessarily larger and the difference with the old design is not very noticeable. Please remove the label before merging so that it looks more minimalist like the one in the first image.

Gabrielxd195 avatar Aug 21 '24 00:08 Gabrielxd195

I tried this, but the only problem is the "W/D" label next to the knob, because visually it makes the plugin space look unnecessarily larger and the difference with the old design is not very noticeable. Please remove the label before merging so that it looks more minimalist like the one in the first image.

Wholeheartedly agree (but please don't thumb up yourself). The label honestly shouldn't be in such a place where it's a convenient knob but as i said in a mirrored bigger knob inside the fx window.

The real beneft coming from this PR is actually there only with the size proposed at the beginning, and it's pretty clear how effective it gets. You should still be able to distinguish the 2 knobs as one starts from the center while the other doesn't, and through tooltips. The difference isn't quite as effective with labels added back in

Before vs after. A simple bar with at most w/d knob is also usually the default behaviour afaik in most daws.

Screenshot 2024-08-16 151622

RoxasKH avatar Aug 22 '24 20:08 RoxasKH

@RoxasKH Actually without the labels the change is quite noticeable, look at all the space that is saved, it looks more modern. I think that all labels should be optional in LMMS, because that is why we have tooltips.

Gabrielxd195 avatar Aug 22 '24 21:08 Gabrielxd195

Tooltips are for additional information, and are not meant to be the sole way of distinguishing UI elements. Labelling elements with tooltips is fine if the elements have sufficiently distinct and suggestive icons, but LMMS has a huge number of near-identical knobs.

Omitting labels hampers feature discovery, as the user has to hover over the appropriate knob even to know that a feature exists. Users familiar with other DAWs, who might reasonably anticipate a particular feature to exist, still have to hover over each knob to find it. Even experienced LMMS users, who know what features are available, have to memorise which knob is which or face the same problem.

Here's some relevant guidance from an article on tooltips by a UX company:

Don’t use tooltips for information that is vital to task completion.

Users shouldn’t need to find a tooltip in order to complete their task. Tooltips are best when they provide additional explanation for a form field unfamiliar to some users or reasoning for what may seem like an unusual request. Remember that tooltips disappear, so instructions or other directly actionable information, like field requirements, shouldn’t be in a tooltip. (If it is, people will have to commit it to their working memory in order to be able to act upon it.)

Surely the name of the knob isn't "additional information" or "reasoning"? And I would consider "which knob does what" to be fairly vital information when trying to do something, that users shouldn't have to memorise.

I realise this is just two knobs for now (the screenshots don't show the decay knob), but it already has minor issues with feature discovery and distinguishing controls, and would quickly degrade the usability of the program if taken as a precedent. While aesthetics should of course be considered, they shouldn't be prioritised over UX.

DomClark avatar Sep 03 '24 23:09 DomClark

Looks great, I would love to test this! Can I get a little summary of everything so far, and points that are worth testing?

bratpeki avatar Sep 04 '24 09:09 bratpeki

Knob labels are back. I honestly prefer the look without them but it seems I am in the minority on this. image

Definitely keep the labels, it makes for better UX.

bratpeki avatar Sep 04 '24 10:09 bratpeki

Tooltips are for additional information, and are not meant to be the sole way of distinguishing UI elements. Labelling elements with tooltips is fine if the elements have sufficiently distinct and suggestive icons, but LMMS has a huge number of near-identical knobs.

Omitting labels hampers feature discovery, as the user has to hover over the appropriate knob even to know that a feature exists. Users familiar with other DAWs, who might reasonably anticipate a particular feature to exist, still have to hover over each knob to find it. Even experienced LMMS users, who know what features are available, have to memorise which knob is which or face the same problem.

Here's some relevant guidance from an article on tooltips by a UX company:

Don’t use tooltips for information that is vital to task completion.

Users shouldn’t need to find a tooltip in order to complete their task. Tooltips are best when they provide additional explanation for a form field unfamiliar to some users or reasoning for what may seem like an unusual request. Remember that tooltips disappear, so instructions or other directly actionable information, like field requirements, shouldn’t be in a tooltip. (If it is, people will have to commit it to their working memory in order to be able to act upon it.)

Surely the name of the knob isn't "additional information" or "reasoning"? And I would consider "which knob does what" to be fairly vital information when trying to do something, that users shouldn't have to memorise.

I realise this is just two knobs for now (the screenshots don't show the decay knob), but it already has minor issues with feature discovery and distinguishing controls, and would quickly degrade the usability of the program if taken as a precedent. While aesthetics should of course be considered, they shouldn't be prioritised over UX.

I agree But it's not just about "aesthetics", slimmer effects improve usability of the drag and drop reordering fx chain, and help to get a better understanding of an fx chain at a glance. This is obviously still an improvement over the initial design, and i'm all for it, but i believe from my screenshot it's kind of easy to tell how less effective the rework becomes once label are added. It goes from 27px saved to 14, basically a 50% reduction in height reduction. The first implementation almost slims the original 57px it by 50%, compared to just 28% with added labels.

About "Users familiar with other DAWs, who might reasonably anticipate a particular feature to exist", to my knowledge and after a quick research, users familiar with other DAWs would most likely not anticipate this. As far as i've seen most daws actually don't have additional controls in the effects chain list aside from active/inactive, but just keep them inside the effect window, usually with a topbar. Unless they integrate the effect window inside the chain, like ableton does afaik The only DAW which has something like that is FL Studio, which only has 1 knobs, Wet/Dry, which is mirrored in the effect window inside a topbar. The thing is, controls here should be there just for convenience, and i believe LMMS will also need in the future to move them or at least mirror those controls inside the effect window. It makes sense, even just from a consistency standpoint, as this is already how it's done for instruments, with convenience controls on the track that are also in the instrument window. Once mirrored, convenience controls could possibly not need a label, as the main control inside the effect window would, at least that's how i see it. It's also to be noted that mos DAWs don't have such an height for FX chains elements, and it's probably for the same reasons.

All of this to just say either way it's an improvement, i'm ok with it, and there's always room for possible changes later

RoxasKH avatar Sep 04 '24 18:09 RoxasKH

@DomClark It should be noted that in this specific case there is only one knob with a very basic function, with the option to "restore" it to default value and with the tooltips with the information of the knob with the mouse movement, so new users already have enough information to know what that knob does. Adding the label makes this pr pointless because I guess the idea is to simplify the effects chain and make it more intuitive. Also there are cases in music composition and sound design where you need to add many plugins to get the desired sound, and this improvement in the effects chain (without the label) makes the workflow better, because now you can see the plugins at a glance and not have to keep scrolling.

Gabrielxd195 avatar Sep 04 '24 20:09 Gabrielxd195

If we want to keep the "W/D" label, maybe it could be placed to the side of the knob and rotated 90 degrees? It shouldn't impact the vertical height that way. I'm not sure how nice it would look, but might be worth trying.

The auto quit knob probably shouldn't be in the EffectView widget at all since it provides such a specific functionality that few people use, though moving it to a topbar in the effect window as @RoxasKH mentioned could be done in a later PR.

messmerd avatar Sep 04 '24 21:09 messmerd

I think the best solution might be to make the list widget of the plugins more flexible so that it can deal with effect widgets that render themselves at different sizes.

The effect widget could then be implemented in such a way that it checks a setting which decides whether users want to see the labels or not. Seeing the labels should be the default settings so that users can learn the interface. Once they know it they could decide to switch to a "compact mode" in the settings where the labels are not shown and not much space is used.

michaelgregorius avatar Sep 05 '24 15:09 michaelgregorius

Another point that i would like to mention here is that since SlicerT is now larger, the current EffectView looks like this.

image

Address this and the other concerns and we good.

Rossmaxx avatar Sep 28 '24 07:09 Rossmaxx

Another point that i would like to mention here is that since SlicerT is now larger, the current EffectView looks like this.

image

Address this and the other concerns and we good.

Addressing this here might be out of scope.

sakertooth avatar Sep 28 '24 12:09 sakertooth

Addressing this here might be out of scope.

It could be extended to include the scope, as it's just a matter of making the incoming layout respond to horizontal resizing, which i believe is far easier because only the effect name widget should be resized and the rest are of fixed sizes, and it's similar to the work done on the mixer ui recently.

Rossmaxx avatar Sep 28 '24 14:09 Rossmaxx

IMO fixing the effect view is out of scope of this issue. The ideal fix would make the effect list view flexible, e.g. by using a layout to manage the effects, and would make the effect view scalable so that it can render at arbitrary widths. This is a large change though.

michaelgregorius avatar Sep 28 '24 15:09 michaelgregorius

If everyone says so, alrite. Btw when is the name button fix coming? We've been waiting for this for some time.

Rossmaxx avatar Sep 28 '24 15:09 Rossmaxx