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

LateNight :: repair [Channel2] xFader button, use button templates #1881

Merged
merged 3 commits into from
Nov 3, 2018

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 2, 2018

https://bugs.launchpad.net/mixxx/+bug/1801363
This is just started as a quick fix for the scenario when a user switches to 2 decks and the xFader orientation of deck 1 or 2 differs from default, like 1=Right and 2=Center. In that case the xFader buttons would stay visible in 2-decks mode until decks 1/2 are assigned default xFader side again.

Just added attribute to PushButton. This is necessary since scalemode="STRETCH_ASPECT" was added to most buttons in #1858 but buttons wouldn't show up without Size attribute.

Edit I continued #1858 and replaced inline PushButtons with templates wherever it makes sense (almost everywhere, except some special cases, and except in sampler templates as that's fixed in #1840 already).

Edit2 This also fixes blurred beatgrid icons next to the waveforms.

@rryan
Copy link
Member

rryan commented Nov 3, 2018

On macOS I don't see any issue with the xfader orientation buttons (even at its minimum size)

screen shot 2018-11-02 at 11 29 16 pm

@rryan
Copy link
Member

rryan commented Nov 3, 2018

Hm, just noticed the star rating widgets don't render or work (they don't seem to receive any mouse input).

@rryan
Copy link
Member

rryan commented Nov 3, 2018

Hm, just noticed the star rating widgets don't render or work (they don't seem to receive any mouse input).

Nevermind, they do work (you need a track loaded). The mouse input is weirdly offset though.. clicking on star N causes a rating of N+1 to be set. And the rendering is cut off.

screen shot 2018-11-02 at 11 35 17 pm

@ronso0
Copy link
Member Author

ronso0 commented Nov 3, 2018

This is just a quick fix for the scenario when a user switches to 2 decks and the xFader orientation of deck 1 or 2 differs from default, like 1=Right and 2=Center

The issue doesn't exist with 4 decks (no scalemode="STRETCH_ASPECT" there), but with 4 decks those buttons should look blurry when the skin is auto-sclaed to 4K. Will update those buttons, as well.

@ronso0 ronso0 changed the title LateNight :: repair visibility of [Channel2] xFader orientation button LateNight :: repair [Channel2] xFader button, use button templates Nov 3, 2018
@ronso0
Copy link
Member Author

ronso0 commented Nov 3, 2018

Nevermind, they do work (you need a track loaded). The mouse input is weirdly offset though.. clicking on star N causes a rating of N+1 to be set. And the rendering is cut off.

I didn't touch that, does it happen without this PR, too?

@ronso0
Copy link
Member Author

ronso0 commented Nov 3, 2018

I carried on with those HiDPI fixes started in #1858 and moved from inline PushButtons to button templates.

Just in case we're in a hurry for 2.2, and no one finds the time to thoroughly review this, the essential quick fix for the orientation button(s) 142f829 and ac55845 can be cherry-picked

@ronso0 ronso0 force-pushed the latenight-xFader-orientation-fix branch from 0586d68 to 931728f Compare November 3, 2018 18:18
@ronso0
Copy link
Member Author

ronso0 commented Nov 3, 2018

(rebased, cleaned up commit messages)

@rryan
Copy link
Member

rryan commented Nov 3, 2018

Sorry for not being clear -- I wasn't saying your PR caused the star rating widget issue :) it happens without this PR.

@rryan
Copy link
Member

rryan commented Nov 3, 2018

Ok, sorry for being dense. It works! LGTM

@rryan rryan merged commit 21b272d into mixxxdj:2.2 Nov 3, 2018
@ronso0 ronso0 deleted the latenight-xFader-orientation-fix branch November 3, 2018 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants