jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
1k stars 224 forks source link

Update left-side section in gui #1729

Closed henkdegroot closed 3 years ago

henkdegroot commented 3 years ago

Has this feature been discussed and generally agreed? Feauture explains itself. Was suggested to perform in #1658.

Describe the solution you'd like Improve the left side layout:

current-layout

New Layout with Reverb at max. height: left-side-max-size

henkdegroot commented 3 years ago

Please assign this issue to me so I can create a PR for this.

hoffie commented 3 years ago

Thanks!

Please assign this issue to me so I can create a PR for this.

Done. BTW, it's good to have assignees for issues to make it clear that someone is working on them and to avoid duplicate work. I believe, when it's clear that a specific thing should be done (I think @pljones hinted that this would be wanted) or is associated with limited amount of work (this issue does not seem too huge to me), then I think it's perfectly OK to open the issue and document your intention to work on it (in the issue text, or, as a comment for existing issues).

henkdegroot commented 3 years ago

@hoffie , for me it makes sense to wait a little with the PR until the final 3.8.0 has been done? So that my PR has less impact on the merge.

hoffie commented 3 years ago

I think work on PR creation and review can continue in this phase. Reviews might be a bit slower and we have to be sure not to merge before the final release. But other than that... it's ok to go on, IMO. :)

pljones commented 3 years ago

Another point... Why do the Delay and Buffer LEDs go "flat" but the Input LEDs stay "fancy"..? It looks a bit wrong. Should be one way or the other. (I'd go for "fancy" consistently - but it might have been for a good reason like accessibility, which wins.)

Not sure if it's worth adding to this or to another small issue?

dcorson-ticino-com commented 3 years ago

Personally I don't like this.
My first try when I made that change was to do just this, but I found it was uncomfortable when the window is tall. Just like trying to read text on the screen when the font size is very large. Yes you can, but it is much easier when the size is more reasonable. The input meter has the same problem, but there I decided to not change the status quo. I think the reverb slider should have a maximum size and not use all the space when the window is tall. I have no problem with making that maximum size bigger than it is now, however.

Note that on rc1 the clientdlg leds are all fancy. That means they do not have the accessibility features. Is that what we want ? In Settings, Audio/Network, the leds are, however, flat with the accessibility features. EDIT: I just saw that the leds are skin dependent, fancy for the fancy skin and flat for the others.

henkdegroot commented 3 years ago

Not sure if it's worth adding to this or to another small issue? I just saw that the leds are skin dependent, fancy for the fancy skin and flat for the others.

Indeed the Buffer and Delay LED follow the skin setting here. What do you mean with "accessibility features"? I believe there is a discussion #1466 open related to accessibility, if you referring to that, perhaps a change to the Buffer/Delay LED needs to be addressed in that.

dcorson-ticino-com commented 3 years ago

The "accessibility features" are for the color blind, a dot in the yellow led and an x in the red led. These already exist in the non-fancy leds.

pljones commented 3 years ago

The "accessibility features" are for the color blind, a dot in the yellow led and an x in the red led. These already exist in the non-fancy leds.

Just to be clear, these features are only required for the on/off indicators. The level indicators essentially have their own accessibility - in the number or length of the lit section.

I think the reverb slider should have a maximum size and not use all the space when the window is tall. I have no problem with making that maximum size bigger than it is now, however.

Ah right, okay, that makes some sense. I'd still like the "top" icon pinned to the top and the "bottom" LEDs pinned to the bottom. Having the Reverb vertically centred and spreading up to some maximum size (with expanding fillers above/below) would work, too.

henkdegroot commented 3 years ago

Just to be clear, these features are only required for the on/off indicators. The level indicators essentially have their own accessibility - in the number or length of the lit section.

Should I include enhancement of the fancy LEDs for display in this section to have similar accessibility indicators as the flat ones?

I will add an new screenshot with a nice maximum size for the reverb.

henkdegroot commented 3 years ago

Here are two samples:

Min. Height: left-side-min-size

Max. Height (reverb slider approx. twice the height): left-side-max-size

pljones commented 3 years ago

I could go with the reverb being much taller. But I never actually use it, it's just screen decoration ("clutter" ;) ) to me...

henkdegroot commented 3 years ago

It seems to be very hard to convince Qt how to "space" the different elements here. The Slider part is between two vertical spacers, and they seem to take precendence when you enlarge the application window. I tried working with strecht factor and different height settings, but no luck. At least is looks okay to me like this and keep the logo nicely at the top. I will create the PR with these settings.

@pljones , what about the "fancy LEDs" for Delay and Buffer? Should I update the RED and YELLOW with a X and DOT? delay-buffer-fancy

pljones commented 3 years ago

It seems to be very hard to convince Qt how to "space" the different elements here.

OK, not to worry too much.

@pljones , what about the "fancy LEDs" for Delay and Buffer? Should I update the RED and YELLOW with a X and DOT?

Oooh, that would be nice.