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

WSpinny: allow to toggle cover art at runtime #4565

Merged
merged 3 commits into from
Dec 17, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Dec 11, 2021

https://bugs.launchpad.net/mixxx/+bug/1883362

This allows to have only one spinny instance per deck, which in return allows to simplify the skins (and probably decreases memory footprint a bit).

  • simplify LateNight
  • simplify Tango

@github-actions github-actions bot added the ui label Dec 11, 2021
@ronso0 ronso0 marked this pull request as draft December 11, 2021 23:18
@github-actions github-actions bot added the skins label Dec 11, 2021
@ronso0 ronso0 force-pushed the spinny-cover-toggle branch from 8aabd61 to 46822b3 Compare December 12, 2021 00:39
@ronso0 ronso0 marked this pull request as ready for review December 12, 2021 00:39
@ronso0 ronso0 added this to the 2.4.0 milestone Dec 12, 2021
@@ -176,7 +178,19 @@ void WSpinny::setup(const QDomNode& node, const SkinContext& context) {
size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
}

m_bShowCover = context.selectBool(node, "ShowCover", false);
// Dynamic skin option, set in WSpinny's <ShowCoverControl> node.
if (showCoverConfigKey.isValid()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the config key for showing cover art have to be configurable? Why can't it just be a single CO defined inside Mixxx? (e.g, always [Skin],show_coverart). Its value never varies in the updates you made

Copy link
Member Author

@ronso0 ronso0 Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like this to be configurable because spinny and cover art can also be shown separately, like in Deere (which I'll adjust in a separate PR) (**Edit:**no need for adjustments there): spinnies next to the waveforms always show the cover while the cover can be toggled in the decks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I see

Copy link
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reducing this kind of skin hack is always a good thing! thanks.

@daschuer
Copy link
Member

LateNight works like a charm.

In Tango there is a grayed out element which is grayed out even more if the spinny and the cover-art is disabled:
grafik
Is this broken? What is this for?

At least it is no regression form main. Is this related here?

@ronso0
Copy link
Member Author

ronso0 commented Dec 17, 2021

yeah the Tango settings UX there is not optimal.
What you discovered is that small spinnies are selected automatically if you show the cannel mixer (thus the orange exclamation mark next to the mixer toggle).
The size button gets and additional translucent cover if spinny and cover are disabled. this is all a bit clunky due to how skins work, e.g. no scripting for control combinations like this.

anyway, the skins should look exactly as before.

@daschuer
Copy link
Member

OK so we can merge. Thank you.

@daschuer daschuer merged commit ce2018f into mixxxdj:main Dec 17, 2021
@ronso0 ronso0 deleted the spinny-cover-toggle branch December 17, 2021 17:33
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.

3 participants