Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add range display for tempo slider #3693

Merged
merged 18 commits into from
Mar 13, 2021
Merged

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Mar 11, 2021

A rebased and fixed version of #2942 by @katsar0v.

@ronso0
Copy link
Member

ronso0 commented Mar 11, 2021

Didn't check the fixes, yet, but do you really intend to merge it into 2.3?
If so it would be a LateNight-only feature. I'd be okay with that.

@Holzhaus
Copy link
Member Author

Hmm, I'm fine with merging it into 2.3 as LateNight only. It's pretty useful for controllers that allow switching the rate range on the fly. But I don't insist. There are no conflicts, so we can simply switch the base branch to main if we want to.

@ronso0
Copy link
Member

ronso0 commented Mar 11, 2021

alright, let's put it in 2.3. I was just wondering if that was by mistake because #2942 was started for main.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 11, 2021

I'm wary of including a new feature in 2.3 at the last minute. I'd prefer to wait until this is implemented in every skin before releasing it.

@ronso0
Copy link
Member

ronso0 commented Mar 11, 2021

@Be-ing I think it's okay to have it in LateNight only for now because compared to the other skins it's a bit advanced now anyway (full/medium/mini decks, rate center dot in PaleMoon). Besides, we don't have equal feature sets in all skins, like split/parallel waves in Deere, or the wave toggle in Tango & LateNight.

@Holzhaus
The rate scale was blurred (I remember I fixed that already elsewhere..)
fixed with 01bc848

  • when the slider handle is at max/min it overlaps rate range number

Then you can still look at the other end of the slider. Both numbers are always the same, right?

I think yo were right, also the current rate is shown below the BPM. I think those range labels are only needed once in a while to verify/control the rate range.
And I'd like to push the numbers a bit to the left (and crop the first/last stroke) so the slider is centered and range numbers don't touch the rate buttons.

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

fixed with 01bc848

conflicts with your last commit, will check.
i don't think we should make the range label that small

btw why LateNight: Increase WRateRange width to make "100" fit? How do you set 100%, it's not available in the preferences.
okay, I see https://manual.mixxx.org/2.3/en/chapters/advanced_topics.html#control-[ChannelN]-rateRange

@Holzhaus
Copy link
Member Author

I added the labels to deere, too. I failed to add it to Tango (for some reason they don't show up). Didn't try shade yet, but I agree with @ronso0 regarding feature parity.

@ronso0 there were a lot of merge conflicts in the SVG files and didn't want them by hand, so I just used git checkout --theirs res/skins/LateNight.

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

fixed with 01bc848
@ronso0 there were a lot of merge conflicts in the SVG files and didn't want them by hand, so I just used git checkout --theirs res/skins/LateNight.

I didn't edit the SVG files in 01bc848, just the size and positions.

hmmkay, you were too quick ; ) I already re-applied some tweaks on top of your latest LateNight commit 60616e9 and I hope this works flawlessly now:
c2550a2 unblur rate slider, adjust fonts (+- are a bit bigger)
fb3902b singletons, reduce memory usage and maintenance effort

image

I'll take a look at Tango tomorrow

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

I failed to add it to Tango (for some reason they don't show up)

the pitch slider has a bg color, you need to style the stacked slider/rate display container instead:
image

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

In Shade all controls in deck.xml are positioned in absolute mode with <Pos>, pretty much like the rate range widgets in LateNight. Just make sure to put them before the rate slider so the are underneath and don't block mouse interaction (exactly the opposite as in stacked layouts : )
Labels should also fit easily I think.

@Holzhaus
Copy link
Member Author

In Shade all controls in deck.xml are positioned in absolute mode with <Pos>, pretty much like the rate range widgets in LateNight. Just make sure to put them before the rate slider so the are underneath and don't block mouse interaction (exactly the opposite as in stacked layouts : )
Labels should also fit easily I think.

Done. For some reason they disappear when the mixxx window loses focus though:
Peek 2021-03-12 12-58

@Holzhaus
Copy link
Member Author

I'd prefer to wait until this is implemented in every skin before releasing it.

It's now present in all skins.

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

In Shade ...

Done. For some reason they disappear when the mixxx window loses focus though:

Works here. And no idea how we'd fix it.
maybe it's just setup. Mixxx tooltips appear to be translucent even thoug they have a solid qss color.

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

Great, it also looks good in Tango. Thanks for picking this up and finishing it!
One more graphic change from me 7de1dee make the strokes of pitch scale wider

In Shade / Dark the labels are invisible. Either you inline the font size in the the xml so it's transformed for each theme, or you add it to style-dark.qss, too.
the external way to se it 7f345f9

LGTM when those are merged.
@Be-ing Any notes from you?

@Be-ing
Copy link
Contributor

Be-ing commented Mar 12, 2021

If it's in all skins, I'm okay with releasing it in 2.3.

@Holzhaus
Copy link
Member Author

Holzhaus commented Mar 12, 2021

Great, thanks.

Now that the latenight scale is wider, I need to check how to make the rate text display in front of the scale, not behind it:
Screenshot from 2021-03-12 23-16-12

@ronso0
Copy link
Member

ronso0 commented Mar 12, 2021

That would only be possible by detaching the slider bg from the WSliderComposed and putting it behind everything else.
I don't think that's worth it.
the first & last stroke are indented a bit
image
image
image

please test a2995dd and pick it if you like.

@Holzhaus
Copy link
Member Author

Holzhaus commented Mar 12, 2021

Great, thanks.

Now that the latenight scale is wider, I need to check how to make the rate text display in front of the scale, not behind it:
Screenshot from 2021-03-12 23-16-12

@ronso0 Do we have a way to set Qt::WA_TransparentForMouseEvents or Qt::WindowTransparentForInput for the widget group? Because If I put the rate range group in front of the slider, you can't move the slider with the mouse anymore. I'm can't make it work using QSS.

If there is not way to fix this, we either need to make the scale less wide or we just ignore the issue because values >90% aren't settable via UI anyway.

@ronso0
Copy link
Member

ronso0 commented Mar 13, 2021

need to make the scale less wide

that's a2995dd

set Qt::WA_TransparentForMouseEvents or Qt::WindowTransparentForInput for the widget group

That is an interesting solution I wasn't aware of. That would allow to enlarge the slider surface and ease click/touch interaction
If that works via <TransparentForClicks> it would be handy for all passive widgets like WLabel (and derivatives) and WStatuslight for example, so we'd need to implement it for wbasewidget (or wbasewidget).

@ronso0
Copy link
Member

ronso0 commented Mar 13, 2021

via

setAttribute(Qt::WA_TransparentForMouseEvents, true); works perfectly for Label and WidgetGroup (inheriting from parent would be nice), but unfortunately that approach fails as soon as the transparent group also contains a SingletonContainer (like RateRange in LateNight here).

@Holzhaus
Copy link
Member Author

Okay, then I guess we are ready to merge now. Thanks for helping out with the skin integration!

@ronso0
Copy link
Member

ronso0 commented Mar 13, 2021

That would only be possible by detaching the slider bg from the WSliderComposed and putting it behind everything else.
I don't think that's worth it.
the first & last stroke are indented a bit
image
image
image

please test a2995dd and pick it if you like.

@Holzhaus
Copy link
Member Author

Already picked it. Thanks.

@ronso0
Copy link
Member

ronso0 commented Mar 13, 2021

alright, was wondering why that didn't show up after my comment.

Great. Quick maneuver.

@ronso0 ronso0 merged commit 1a8f8d3 into mixxxdj:2.3 Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants