search

michaelforney / oscmix

Mixer for RME Fireface UCX II
Other
20 stars 4 forks source link

Inverted channel volume slides #6

Closed amaennel closed 6 months ago

amaennel commented 6 months ago

Hi!

First of all great project, thanks for working on this.

While playing around with the web frontend I noticed a small issue with the channel volume sliders.

When using firefox 123, which supports CSS vertical writing on range elements, the sliders have the wrong orientation. When using firefox 116 (which is the current snap version on ubuntu 22.04) , which doesn't support CSS vertical writing on range elements, the sliders have the expected orientation.

I assume you tried adding writing-mode: vertical-lr to the sliders in a version that does not yet support this, but left it in accidentally. This css setting can just be removed I think.

See: https://developer.chrome.com/blog/vertical-form-controls

Another thing I have noticed is that on firefox 116 the channel settings, eq and dynamics do not collapse as they should. The css operator :not was only introduced in firefox 121, so this doesn't work for a vanilla ubuntu installation.

Long story short, I think a minimum browser version requirement should be in the readme, because this sadly is a reality for web dev. I can also make a pull request for these changes if you want.

Cheers, Alex

michaelforney commented 6 months ago

Thanks for bringing up these issues. I've mainly been testing in flatpak chromium, but I did notice some of the issues you described.

I would like to support other browsers, but if it's not easy to do, I don't really want to spend too much time on it.

Hi!

First of all great project, thanks for working on this.

While playing around with the web frontend I noticed a small issue with the channel volume sliders.

When using firefox 123, which supports CSS vertical writing on range elements, the sliders have the wrong orientation. When using firefox 116 (which is the current snap version on ubuntu 22.04) , which doesn't support CSS vertical writing on range elements, the sliders have the expected orientation.

I assume you tried adding writing-mode: vertical-lr to the sliders in a version that does not yet support this, but left it in accidentally. This css setting can just be removed I think.

I was looking at https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range#putting_it_all_together when trying to figure out how to make the range/meters vertical.

I first tried appearance: slider-vertical as suggested, but that produced a warning that told me to use writing-mode instead. writing-mode: vertical-lr seemed to work, so I went with that. But it wasn't supported in firefox, so I added orient="vertical" as well. I did noticed that it inverted the elements, but I didn't know how to fix it.

See: https://developer.chrome.com/blog/vertical-form-controls

Thanks, that's a good reference. It seems that direction: rtl is needed to set the orientation correctly. It looks like that in combination with writing-mode: vertical-lr works in chromium and firefox 122, so I've pushed this change.

I'm not sure about 116, but if there's an easy way to display properly there, too, I'd like to support it.

Another thing I have noticed is that on firefox 116 the channel settings, eq and dynamics do not collapse as they should. The css operator :not was only introduced in firefox 121, so this doesn't work for a vanilla ubuntu installation.

Hmm, ok. I think a lot of the :not logic could be inverted by changing the default to display: none, and setting it to the proper value (not sure what, maybe revert or something?) when the panel button is selected. Thoughts on that?

Long story short, I think a minimum browser version requirement should be in the readme, because this sadly is a reality for web dev. I can also make a pull request for these changes if you want.

Yes, that would be helpful.

Also, since you seem to be more web-savvy than me, maybe you could help with one issue I just couldn't seem to figure out: when you shrink the window vertically, the channel strips start to individually scroll vertically. This is because the volume range seems to have some minimum height. If I set it to height: 100px or something, the channels show up as I intend, but then of course its height is fixed. I can't figure out how to make them fill up the entire height of the channel channel, and shrink down further than their default height.

amaennel commented 6 months ago

I'm not sure about 116, but if there's an easy way to display properly there, too, I'd like to support it.

I can't really think of an easier way, but this is not something I have looked into much. My intention with the issue was also not to have every browser compatibility issue fixed (impossible) but rather to make you aware of it. As there is currently a lot more important stuff to do, I still think that the best way to navigate this is to just provide a Tested in chrome X.X section in the README for now.

Hmm, ok. I think a lot of the :not logic could be inverted by changing the default to display: none, and setting it to the proper value (not sure what, maybe revert or something?) when the panel button is selected. Thoughts on that?

Default value for div is display: block. But that will probably not solve it for Firefox < 121. Again not that it's super important.

Also, since you seem to be more web-savvy than me, maybe you could help with one issue I just couldn't seem to figure out: when you shrink the window vertically, the channel strips start to individually scroll vertically. This is because the volume range seems to have some minimum height. If I set it to height: 100px or something, the channels show up as I intend, but then of course its height is fixed. I can't figure out how to make them fill up the entire height of the channel channel, and shrink down further than their default height.

I'm not entirely sure why, but adding min-height: 0 to .channel-volume solves this. display: flex sets min-height: auto on the flex-items per default so that might have something to do with it. Note that the minimum height of the channels is now implicitly set by min-height: 200px in .channel-strip so they still won't shrink completely.

A bit unrelated maybe: I feel like the WebMidi part isn't getting enough attention in the README. It's pretty cool that you got that working that well, because it allows people to use oscmix even if they aren't particularly linux-savvy. But I only found out that that exists via the forum thread.

michaelforney commented 6 months ago

I'm not entirely sure why, but adding min-height: 0 to .channel-volume solves this. display: flex sets min-height: auto on the flex-items per default so that might have something to do with it. Note that the minimum height of the channels is now implicitly set by min-height: 200px in .channel-strip so they still won't shrink completely.

Thank you! That did the trick.

A bit unrelated maybe: I feel like the WebMidi part isn't getting enough attention in the README. It's pretty cool that you got that working that well, because it allows people to use oscmix even if they aren't particularly linux-savvy. But I only found out that that exists via the forum thread.

I don't really know what to add. Feel free to send a PR if you have some idea.

Closing this, since I believe the original should be fixed with new firefox.