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

Skins: add effect chain menu button to Deere, polish in Tango #12735

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Feb 5, 2024

image
image

@ronso0 ronso0 added this to the 2.4.0 milestone Feb 5, 2024
@github-actions github-actions bot added the skins label Feb 5, 2024
@ronso0 ronso0 added the effects label Feb 5, 2024
@ronso0 ronso0 linked an issue Feb 5, 2024 that may be closed by this pull request
@daschuer
Copy link
Member

daschuer commented Feb 5, 2024

This is how it looks like without a super knob and the menu gear highlighted.
grafik

grafik

I have no strong demand here to change something in the last minute. Just for info what I had in mind when first looking it.

  • The gear looks a bit lost compared to the other controls squeezed to the border.
  • The hover effect does looks not natural, because unlike other buttons the gear has no border.
  • The Deere gear looks bold compared to most other icons.

Please give me a hint when you consider it as ready.

@ronso0
Copy link
Member Author

ronso0 commented Feb 6, 2024

Thanks for taking a closer look (than I did in bright sunshine ; )

I polished the gear icons a bit.

The hover effect does looks not natural, because unlike other buttons the gear has no border.

The gear looks a bit lost compared to the other controls squeezed to the border.

Yeah, I know, but at least its consistent with the Skin Settings toggle. When adding a border we'd need to make the icon smaller and I thnik that's not worth it. Tbh I'm not very motivated to work on Tango anymore, the mirrored layout and dealing with size policies is just a nightmare :| (I'll fix bugs of course)

I think this is ready aka good enough to be merged.
Maybe I'll come back for a bit of polishing.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you for polishing. LGTM.

@daschuer daschuer merged commit cddc33d into mixxxdj:2.4 Feb 6, 2024
14 checks passed
@ronso0 ronso0 deleted the effect-menu-button branch February 6, 2024 08:44
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.

add Effect Chain preset menu button to Deere and Tango
2 participants