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

limit play marker hints text height to 1/3rd or full waveform height #13465

Closed
wants to merge 2 commits into from

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Jul 13, 2024

The text height of the Play marker hints (beats and time until next marker) was limited to a third of the waveform viewer. Increasing the font size beyond that had no effect, until the waveform viewer was made higher.

This was confusing to the user and could be interpreted as a bug. Also, this limit of 1/3rd was arbitrary and not obvious. See this comment from Eve:

https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/.28RFC.29.20show.20beats.20until.20next.20marker/near/449084479

To make the mechanism more clear, I have renamed "Font size" to "Preferred font size" and added a combobox "Text height limit" with options "1/3rd on waveform viewer" and "Full waveform viewer". (This could also have been a spinbox, but I feel this is more clear and I don't see much value in more find grained control, though I am open to adding more options. Maybe 1/2 and 2/3rd?)

@Eve00000
Copy link
Contributor

My apologies, I had no idea that you intended the font to be max 1/3 of the waveform height. I was already used with big font from the first tests that I thought it was a bug
Human habits :-)

@m0dB
Copy link
Contributor Author

m0dB commented Jul 19, 2024

Hi @Eve00000 , you have given this PR some testing, right? If you confirm it is working as expected, maybe @daschuer or @ronso0 can hit the merge button :-)

@Eve00000
Copy link
Contributor

This pr is doing the trick!
Pushing my luck: (maybe m0db has overseen my request); possibility to place the counter left / center / right of the waveform would make it complete

@m0dB
Copy link
Contributor Author

m0dB commented Jul 20, 2024

Hi @Eve00000 . I saw the request, the reason I have not done this is because I was thinking about reserving the left side for beats/time since previous marker, and I need to give this a bit more thought.

@Eve00000
Copy link
Contributor

Oh that would be nice to, past on the left, future on the right.
Thank you, that is all good (even super), it's just the problem with written on-line communication. Things fly by, you can't see a reaction, a nod, an ok.
Sometimes you read st, you want to react, but you need to orden your thoughts and later you forget or can't find the original message.

Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Jan 18, 2025
@ronso0
Copy link
Member

ronso0 commented Jan 18, 2025

So the only remaining change after merging #13953 is the layout fix, right?
And that I already adopted in #13615 (which is ready for final review btw), so we can close this?
(unless you're keen on resolving the conflicts here and give me some headaches in #13615 ; )

@m0dB
Copy link
Contributor Author

m0dB commented Jan 18, 2025

Yes, this can be closed.

@m0dB m0dB closed this Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preferences stale Stale issues that haven't been updated for a long time. ui waveform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants