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

Library layout clean ups #883

Merged
merged 10 commits into from
Feb 13, 2016
Merged

Conversation

22degrees
Copy link
Contributor

Not a bugfix but a bit love to the library section.
Reset the default margins to zero and make it more styleable via qss.
Some commits breaks the skins a little bit. But if required, I would fix it.

Please review.

@daschuer
Copy link
Member

daschuer commented Feb 1, 2016

Thank you for the Pull request. I have just tested it and IMHO some changes are improvements others not. The Buttons are now stacked together which looks odd. Why is the change required?

@daschuer
Copy link
Member

daschuer commented Feb 1, 2016

What happens if you apply a css margin?
Can we add a default margin, that is overridable by css, without breaking existing skins?

@22degrees
Copy link
Contributor Author

Thanks for your feedback.

My personal opinion is:

  • all skin elements have no margins, paddings or spacings by default
  • the designer add these values as required via qss
  • it is easier to set a value via css as to make time-consuming experiments to find the right property for an override

Sure, it is possible to reset the defaults via qss. You have already mentioned it.
But I didn't find any workable solution to override the layout Spacing in the LibrarySection with "qproperty-layoutSpacing: 0;" or the hardcoded spacer.

I agree that the existing skins look odd with these changes, but my skin looks great now ;)
(screenshot attached) and the skins can be quickly fixed.

I make a fix for the existing skins in the next few days.

regards Hendrik

skin roundcorners

@22degrees
Copy link
Contributor Author

I think the existing skins are fixed and they look like before.
Attached you will find screenshots.

latenight

shade

deere

@daschuer
Copy link
Member

daschuer commented Feb 4, 2016

Nice, thank you for the work. LGTM

@esbrandt and others: Any other comments before merge?

Before merge, you need to become a Mixxx contributor.
Please sign: https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ
and comment here when done.

@22degrees
Copy link
Contributor Author

Thanks for review.
Agreement subscribed.

daschuer added a commit that referenced this pull request Feb 13, 2016
@daschuer daschuer merged commit 4d6b95d into mixxxdj:master Feb 13, 2016
@daschuer
Copy link
Member

I would like to add your real name to the contributor list in the AboutBox inside Mixxx.
Is this OK for you?

@22degrees
Copy link
Contributor Author

That's ok!

@daschuer
Copy link
Member

Done:
1b1a5f0

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