wwmm / easyeffects

Limiter, compressor, convolver, equalizer and auto volume and many other plugins for PipeWire applications
GNU General Public License v3.0
6.51k stars 269 forks source link

New App info UI #1427

Closed Digitalone1 closed 2 years ago

Digitalone1 commented 2 years ago

Given the issues related to the current layout, I made a new one for testing.

Schermata del 2022-03-12 23-22-56

@wwmm What do you think? Any hint?

The space should be more optimized with this layout and scrolling doesn't change the app volume (#1211).

@wwmm I take this opportunity to ask another thing. I didn't notice before, but getting rid of GTKMM the locale format is not applied anymore. I see you use ftm library now. Is it intended to not have the locale or you forgot to implement it? Ftm has L key to format to locale.

wwmm commented 2 years ago

What do you think? Any hint?

I think it is fine. I did not see anyway to deal with the scrolling problem without putting the volume slider in the vertical orientation. Something that could also be considered is using a GtkScaleButton. I do not have a reason to keep the volume scale visible all the time. So if you feel it is better with the GtkScaleButton go ahead.

I take this opportunity to ask another thing. I didn't notice before, but getting rid of GTKMM the locale format is not applied anymore. I see you use ftm library now. Is it intended to not have the locale or you forgot to implement it? Ftm has L key to format to locale.

I probably forgot to check this. In the beginning of the port I did not put units in the widgets labels and probably the locale was fine. And as I usually test with English I did not notice the problem.

jannuary commented 2 years ago

Bit of a sketch from me:

when the effects are easy

I think the sliders are somewhat neccessary to be accessible, but I otherwise tried to simplify the layout and enforce a better visual hierarchy: auxillary info is inline with the player's title, and exclude/block/disable/etc would be in the popover next to the mute button.

Digitalone1 commented 2 years ago

I think it is fine. I did not see anyway to deal with the scrolling problem without putting the volume slider in the vertical orientation. Something that could also be considered is using a GtkScaleButton. I do not have a reason to keep the volume scale visible all the time. So if you feel it is better with the GtkScaleButton go ahead.

I think the volume scale button is only good if the volume level is still visible, because if the user has to click a button to see the level, I think it's a regression. Seeing the level and clicking the button to show the slider and change the volume, it's good instead.

I think the sliders are somewhat neccessary to be accessible, but I otherwise tried to simplify the layout and enforce a better visual hierarchy: auxillary info is inline with the player's title, and exclude/block/disable/etc would be in the popover next to the mute button.

Thanks @jannuary. Horizontal volume slider is still a problem because it prevents the correct scrolling. Moreover that layout misses the enable and blocklist commands. Where would you add them? If you want, make another sketch with those commands.

Inline info is a good idea, but arranging the labels horizontally could be tough given the default padding of the GTK4 labels and I don't know how to deal with the separators in the middle. With the default GTK padding, the space between the labels could be very big and not pleasant to see. We could also make a single label and construct a concatenated string. I have to think about that. Anyway binding the label with the parameter is more straightforward from a developer point of view.

Update: No, as I think about it, inline layout seems definitely not good because the information is not contextualized. With the grid we specify the state, the format, the latency, the frequency, etc, while in the inline layout a user could not know what F32LE and 48K means...

jannuary commented 2 years ago

Moreover that layout misses the enable and blocklist commands. Where would you add them?

Given they kinda do similar things as is, they're in a menu near the mute button.

On the label side, it's intended to be a single label with · as a separator. I agree that the labels appear somewhat out of context, although, aside from the format, I don't think this is a large issue.

Not sure what would be a good solution to the sliders though - I think for a mixer-ish UI it's probably best to have large (and as such, fine) sliders and ideally we would just disable the scroll gesture, though no idea how that could be done :/

wwmm commented 2 years ago

Moreover that layout misses the enable and blocklist commands

Unless I misunderstood the sketchy the enable button would be the "dashed square". I am not sure we can make a checkbox that looks like that.

I also think that the triangle would be a menu button tied to a menu where things like the blocklist button could be. This is something that could be done. I do not think the blocklist button needs to be visible all the time. So it is fine to move it to a menu.

jannuary commented 2 years ago

Unless I misunderstood the sketchy the enable button would be the "dashed square".

Oops, yeah, that's supposed to be a symbolic icon of the player ‎😅️

wwmm commented 2 years ago

Oops, yeah, that's supposed to be a symbolic icon of the player

Oh! :smile:

wwmm commented 2 years ago

Update: No, as I think about it, inline layout seems definitely not good because the information is not contextualized. With the grid we specify the state, the format, the latency, the frequency, etc, while in the inline layout a user could not know what F32LE and 48K means...

I partially agree. The latency without a label is not going to have a clear meaning for first time users. Same thing to the number of channels. But the state, frequency and format should be self explanatory without labels for the users that actually care about this kind of information.

I like how @jannuary proposal looks. But it would still have the "scroll problem". Unfortunately the volume scale would have to be put in the vertical. I could never found a way to disable its scroll event.

wwmm commented 2 years ago

Anyway binding the label with the parameter is more straightforward from a developer point of view.

I agree. The information we display there comes asynchronously from different signals. Putting all of them on the same label may be a little annoying...

Digitalone1 commented 2 years ago

I'm against hiding commands in a menu. It's already being done in the header bar, we have space in the application tab, it's better to use it.

When I see the application list, I'd like to see every information without having to click to show things. If the volume button does not show the volume level, it's useless.

Hiding the blocklist command is useless too because I'd like to know which application is excluded. If I don't want to see an excluded app, I toggle the relative switch in the blocklist menu in the footer bar.

I partially agree. The latency without a label is not going to have a clear meaning for first time users. Same thing to the number of channels. But the state, frequency and format should be self explanatory without labels for the users that actually care about this kind of information.

I like how @jannuary proposal looks. But it would still have the "scroll problem". Unfortunately the volume scale would have to be put in the vertical. I could never found a way to disable its scroll event.

The problem is that if we show things horizontally, the vertical space is reduced. If it's reduced, the volume vertical scale is so little that's useless. We can't make it horizontally because of the scroll issue, so the last option would be putting an horizontal spinbutton somewhere for the volume.

Or maybe a volume button, but with a syncronized label to show the level. In this case I wonder if the scale will move the neighbor widgets or it will appear over them. Because if it will change the size, we can't do it vertical because the app row will change the height.

wwmm commented 2 years ago

What do you think? Any hint?

@Digitalone1 looking at your image again I think it would be nice to keep the mute button as a toggle as it is now. The functionality is obviously the same with a checkbutton. It just doesn't feel as interesting to the eyes.

I imagine you changed it to a checkbutton because with the slider in the vertical there won't be enough vertical space to do the same we do in the horizontal. Tough call indeed.

Digitalone1 commented 2 years ago

Unfortunately the next week I don't have so much time to dedicate to EasyEffects.

Anyway I leave here some points to resume. Feel free to add suggestions if don't agree.

wwmm commented 2 years ago

Aside the states running, stopped, etc., which are self explanatory for the app, other parameters should have a label that contextualize their meaning (format, frequency, latency...).

We can keep a label for them. My point was only t that hey are not super dependent on a label for first time users like in the latency case.

Digitalone1 commented 2 years ago

@wwmm do you know if there's a way to limit the size of a widget? As I remember, there's no such thing in GTK4 so the widget occupy its size or it can be expanded to fit all the space. We can force a fixed size, so the horizontal slider covers only a part of the app row.

An hack we could apply is to use an horizontally expanded 3 column homogeneous grid, so the slider will fit only 1/3 of the row, and the rest can be used to show other widgets.

wwmm commented 2 years ago

do you know if there's a way to limit the size of a widget?

I did not look at this in gtk4. But I think labels should still accept a fixed amount of characters in their width. The question is how this approach will look in different screen sizes. So maybe your second idea is better.

An hack we could apply is to use an horizontally expanded 3 column homogeneous grid, so the slider will fit only 1/3 of the row, and the rest can be used to show other widgets.

Our bottom panel is implemented with a GtkCenterBox that does exactly that https://github.com/wwmm/easyeffects/blob/6a64218e96018d0e53a8fa1247dc3ef3a6bb1a0a/data/ui/effects_box.ui#L22.

Digitalone1 commented 2 years ago

@wwmm @jannuary what do you think?

Schermata del 2022-03-19 23-46-17

Schermata del 2022-03-19 23-46-50

I'd like to modify the slider value to show some space padding (like plugin in/output level sliders) and the % sign.

wwmm commented 2 years ago

what do you think?

Humm... I think I prefer the approach we currently use because the stream information is centered in the row. In the image above once the width is large there are many things packed together in the left when there is plenty of space in the middle.

If the idea is keeping the slider in the horizontal direction but smaller so that it is easier to scroll the list I think it will be better to keep our current approach but forcing the slider maximum width to be the same of the block with the stream information. In other words it would be better to just shrink it while keeping the current position instead of putting it to the right with a very smaller size.

Digitalone1 commented 2 years ago

Humm... I think I prefer the approach we currently use because the stream information is centered in the row. In the image above once the width is large there are many things packed together in the left when there is plenty of space in the middle.

So the problem is only the empty space in the middle or also the things "packed"? Because if it's only the former, I can just center the whole block. Should be easy. I can make a test screenshot later.

If the idea is keeping the slider in the horizontal direction but smaller so that it is easier to scroll the list I think it will be better to keep our current approach but forcing the slider maximum width to be the same of the block with the stream information. In other words it would be better to just shrink it while keeping the current position instead of putting it to the right with a very smaller size.

The slider would take the majority of the screen size (in most of cases, except very large screen which are rare) even if it takes the same width of the stream info grid. If the target is to facilitate the scroll, the previous layout is better because it takes only the 25% of the horizontal width (with the hack of 4 homogeneous column grid with the first 3 spanned). It was taken in consideration to completely hide it behind a button which is not good in my opinion, shrinking it at 25% in the right side could be a good compromise. The user would have the 75% of the window to scroll, in most cases greater than the size of the layout with the slider restricted to the width of the stream info grid.

I was skeptical about the horizontal stream info, even because the example by jannuary was a single label. But having tested the layout shown before putting the labels in the same box, swapping the dim class on the data and setting a specific margin between different info feels much better. Do you prefer the grid?

Digitalone1 commented 2 years ago

Schermata del 2022-03-20 11-15-01

Or with the app icon aligned to the left:

Schermata del 2022-03-20 11-29-09

Personally I prefer the latter.

Digitalone1 commented 2 years ago

Comparison with wide and low width windows:

Schermata del 2022-03-20 11-34-09

Schermata del 2022-03-20 11-35-26

Slider is too tiny for my taste in low width, maybe better changing the grid to 3 homogeneous columns, so it takes 33% and 66% is still available for the scrolling.

@wwmm what do you think? I prefer this layout and the stream data in one only row feels better than the current grid. Can't do better than that. Let me know.

jannuary commented 2 years ago

I don't think squeezing the volume slider to the point where it's impossible tiny is a good solution to the problem, to be honest.

Otherwise, this is not bad - I like the player info, though I wouldn't necessarily put Enable/Exclude in spotlight.

Digitalone1 commented 2 years ago

In this post I describe what personally is not good about the current layout.

Schermata del 2022-03-20 11-43-23

I don't think squeezing the volume slider to the point where it's impossible tiny is a good solution to the problem, to be honest.

@jannuary You're right, this is a limitation of GTK. If we could specify a fixed size would be better, but the only way to control it in our situation is to make a hack like the grid homogeneous column. Maybe could be better at 1/3 (33%) or 2/5 (40%) of the horizontal width (in the screenshots I posted was 25%).

Otherwise, this is not bad - I like the player info, though I wouldn't necessarily put Enable/Exclude in spotlight.

Where would you put them? I thought putting them in the same section was good because they control how EE handles the stream.

Digitalone1 commented 2 years ago

This is the slider at 2/5 (40%).

Schermata del 2022-03-20 12-42-10

Schermata del 2022-03-20 12-42-40

The size above is the lowest before GTK don't shrink anymore and the horizontal scrolling is enabled (and the horizontal scroll does not work, this is another issue to resolve).

Anyway the slider seems to have a reasonable size and quite 60% of horizontal width remains to scroll.

wwmm commented 2 years ago

I don't like Add the blocklist label. I think we need something better to describe that the application should not be processed by EasyEffects even if the Process All option is enabled. I though Exclude, but maybe there's something better. I'm open to suggestions here.

I have no complaints about this change. The new label is shorter and gets the job done. This is good.

The enable switch is not intuitive at first sight. We do know what it does because we use the app by years, but could be confusing for the new user. I don't care about Gnome Circle, but they were right about that. We need something to tell the user what it does and the check button with the label is good in doing this.

While I agree with the "not intuitive" part in my opinion the checkbuttons do not feel a superior solution from a visual point of view. Specially when the pack of labels with stream information is super close to them. I wonder how it would look with a GtkToggleButton instead. It is not the end of the world to remove the GtkSwitch. But it made the enable button "special". Now it visually has the same importance of the exclude button what again, in my opinion, is weird. But it is not the end of the world...

What I really do not like is this new volume scale position. Honestly between this and having it hidden in a menu I prefer the menu. Let me take a look again at the "scroll situation". I've got close to a solution in the last attempt. We just have to find a way to make gtk to completely ignore the GtkScale when it comes to scroll events and sending them to the parent widget.

Digitalone1 commented 2 years ago

What I really do not like is this new volume scale position. Honestly between this and having it hidden in a menu I prefer the menu. Let me take a look again at the "scroll situation". I've got close to a solution in the last attempt. We just have to find a away to make gtk to completely ignore the GtkScale when it comes to scroll events and sending them to the parent widget.

Okay, let me know. If that scroll issue couldn't be solved, I could test with the GTK volume button and adding a label to show the volume level.

wwmm commented 2 years ago

I wonder if we shouldn't consider using a GtkSpinButton to handle the volume instead of the GtkScale. The scale is more elegant in this task but as it is inside a list its scroll conflicts with the list scroll. Maybe this is a sign we should stop fighting against the toolkit and using another widget. Specially if we are going to change buttons and their positions along the way anyway. Maybe with the new approach a spinbutton will look good in this task.

Digitalone1 commented 2 years ago

No problem. The next week I will do other attempts with:

For stream info data, I will try both grid and horizontal labels.

Digitalone1 commented 2 years ago

@wwmm @jannuary What do you think? Do not pay attention to wrong dimmed long description, it's a known gtk4 bug, already reported.

Schermata del 2022-03-27 00-34-52

Schermata del 2022-03-27 00-59-22

To me it's OK.

wwmm commented 2 years ago

Now that I can see the togglebutton and the checkbutton side by side I have the feeling that the togglebutton may be too heavy on the eyes in this location. Or at least it seems so with the dark theme. So it is probably better to keep both as a checkbutton.

While I agree that labels for the information fields will help new users now that they are all on the same line somehow it feels we have too much labels there. Maybe it is because I do not use dark themes. But I still think that visually this line would look better only with the values instead of having words like format or rate together.

I think I need to some time to meditate on this... I will pin this issue so it gets more visibility and maybe more ideas. Although we would be fixing the issues that were pointed by some people I still have the feeling that visually we will be giving one step back.

Digitalone1 commented 2 years ago

My point of view is that every layout I tried it's better than the current one for the reasons I already explained.

I have in mind another one with the switch button and the volume button. I'll test it.

Anyway I think this afternoon I'll make the merge request, so you can test it on your own system with the light theme and if you have a better idea, you can start modifying from there.

That's an oddity that I noticed with the current layout. We don't have any sensitive function for the volume slider, but when the mute button is toggled, the slider is set unsensitive. This does not occur with the spin button, I had to add the sensitive functions to make it work that way. But the slider didn't have them, maybe it's an automatic behavior of GTK4 with slider binded to volume parameters?

wwmm commented 2 years ago

every layout I tried it's better than the current one for the reasons I already explained.

As I said before I 100% agree with the blocklist button renaming. The other changes have some disadvantages(in my opinion):

Just to clarify things it is not like I think our current layout is good. But with the exception of the blocklist button renaming I still do not feel we will be giving a step ahead the other changes. That is why I think we should put more thought on this.

This does not occur with the spin button, I had to add the sensitive functions to make it work that way. But the slider didn't have them, maybe it's an automatic behavior of GTK4 with slider binded to volume parameters?

That is weird. If I am not mistaken we have bindings for spinbutton sesitivity in the Bass Enchancer and the Exciter window. The same approach should work here too.

wwmm commented 2 years ago

I will make a question in GNOME Discourse about the scroll events on the GtkScale. Maybe a gtk developer will be kind enough to tell if there really isn't a way to handle this situation.

Digitalone1 commented 2 years ago

No problem @wwmm, I will make the merge request so you can test on your system anyway.

I made a new one with the switch. If I have time I will test also the volume button.

Digitalone1 commented 2 years ago

Schermata del 2022-03-27 16-55-23

wwmm commented 2 years ago

Question done https://discourse.gnome.org/t/gtk4-how-to-completely-remove-a-gtkscale-from-scroll-events/9298. Let's see what happens.

wwmm commented 2 years ago

I made a new one with the switch. If I have time I will test also the volume button.

It seems better that the other one to me.

I have to admit I am having a hard time accepting the idea we will probably have to remove the horizontal scale. It looks so nice there...

Digitalone1 commented 2 years ago

That is weird. If I am not mistaken we have bindings for spinbutton sesitivity in the Bass Enchancer and the Exciter window. The same approach should work here too.

Understood why, I removed

<property name="sensitive" bind-source="mute" bind-property="active" bind-flags="sync-create|invert-boolean" />
Digitalone1 commented 2 years ago

With the Volume Button:

Schermata del 2022-03-27 18-05-27

There are some issues:

Another thing to try if we don't want to give up with GtkScale is considering width-request property. Never used it, but maybe it can be useful to set a fixed width for the scale rather than having to use hacks like ones tried previously with grids.

wwmm commented 2 years ago

There are some issues:

I agree. The scale button is not going to help. For some reason I was hoping it would provide some kind of integration for the mute button out of the box. If it is not the case there isn't much point in using it in our situation.

I compiled your branch here and with a light theme it gives a better impression(in my opinion). So we definitely have to try both versions to have a better idea about how the solution is going to look like Screenshot from 2022-03-27 13-28-11

. But I still think it would look way better if the information fields did not have the field name label beside them. In short if they were like @jannuary did in his proposal. I know it is not friendly to new users. I totally agree with that. But visually it hurts to see this chain of labels alternating between dim and normal font. We put a lot of effort in making a manual. I think we can delegate an explanation for these fields there and having just the values in the window. This already had to be done in the bottom panel.

I also wonder if it would not be better to put the Exclude checkbutton above the spinbutton and the mute button.

wwmm commented 2 years ago

In any case let's wait a little more to see if someone from gtk answers my question. If possible I would like to keep the horizontal GtkScale.

wwmm commented 2 years ago

In short if they were like jannuary did in his proposal

But not necessarily in the same line as the app title. Depending on the situation the app title can be long. I meant to say keeping them at the bottom line but without the labels with the information name,

wwmm commented 2 years ago

Depending on the situation the app title can be long.

Or maybe not. I have the feeling I have seen long names in the past. But I can't remember with which app....

wwmm commented 2 years ago

The fun fact is that the "scroll problem" also happens with the SpinButton :smile:. The difference is that the spin button does not need a large horizontal size to be usable. So it is easy to just move the mouse pointer somewhere else.

Digitalone1 commented 2 years ago

No problem @wwmm. You can modify it, I don't think having time in the next days.

Digitalone1 commented 2 years ago

I know it is not friendly to new users. I totally agree with that. But visually it hurts to see this chain of labels alternating between dim and normal font.

No problem. Anyway if the issue is the dim class alternation, you can also try to remove the class. Post a screenshot in case you modify it.

I also wonder if it would not be better to put the Exclude checkbutton above the spinbutton and the mute button.

Okay, I can't do it now, please post a screenshot to show how it seems.

Digitalone1 commented 2 years ago

Anyway, if the volume spinbutton will be merged to the master, we should also format the text to add the % sign.

wwmm commented 2 years ago

No problem. Anyway if the issue is the dim class alternation, you can also try to remove the class.

They are part of the issue. But I think my main problem with all those labels on the same line is that it gets a little harder to quickly identify a value that is in the middle. I think it is more straightforward and elegant to have only the values with the dim class applied. But again it is my opinion.

Anyway, if the volume spinbutton will be merged to the master, we should also format the text to add the % sign.

I agree.

Someone answered my question on Gnome Discourse but no word from a gtk developer yet. Although I can see the proposed workaround working it seems an overkill to me. It is easier to use a spinbutton.

wwmm commented 2 years ago

No answer from a gtk developer on Gnome Discourse and we have to move on. So I have updated the master branch with a combination of the different ideas that we have discussed so far Screenshot from 2022-03-31 13-04-49

wwmm commented 2 years ago

I am trying to do a boxed list like the one in @jannuary proposal but somehow it is not working. The strange part is that Adwaita docs suggests it is super simple to do https://gnome.pages.gitlab.gnome.org/libadwaita/doc/1.1/boxed-lists.html

wwmm commented 2 years ago

but somehow it is not working

It works. But there is so little contrast in the default configuration that it almost can not be seen... Screenshot from 2022-03-31 15-06-09