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

Style tweak, dimming highlight color #528

Merged
merged 4 commits into from
Feb 21, 2020

Conversation

davidlatwe
Copy link
Collaborator

What's changed

  • Comment out QHeaderView::section's color attribute so one could customize it
  • Dimming selection highlight color so the icons could pop-up, having more contrast

Before

9986041b-fa52-44ba-bc62-fd64defeee5d

After

3c8e4214-919b-47ea-bfff-9fa2a790d7bf

Custom header section text color

image

Motivation

Although I did think about overriding styles from the config, but then I thought it could become a bit harder to develop GUI in style that everyone could adopt since what other sees would be different from mine. So I decide to push the change I need and have a open discuss to see if everyone feels okay with it.

It's not much but I think it could improve the overall color contrast for seeing icons and other things that are a bit more important than selections. Bright icon friendly style. ☺️

@@ -4,6 +4,11 @@
font-family: "Open Sans";
}

/*
@ Selection highlight: #40576D
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was like a quick access for testing color, since qss doesn't support declaring variables. Could be removed if you want it more cleaner.

Copy link
Collaborator

@tokejepsen tokejepsen left a comment

Choose a reason for hiding this comment

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

I like it!

Comment on lines -508 to -514
QHeaderView::section
{
background-color: #3A3939;
color: silver;
padding-left: 4px;
border: 1px solid #6c6c6c;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This section was duplicated, there are one exact definition with more attributes down below this style.qss file.

@mottosso
Copy link
Contributor

Nice one :)

@BigRoy
Copy link
Collaborator

BigRoy commented Feb 17, 2020

Looking good! At first it did give me the feeling that somehow it was "disabled" because of the dimmed color but likely it just needs some getting used to.

What does it look like when a row is disabled? Or the widget is disabled?

I always wondered whether we should make the Alternating Row color slightly subtler and easier to the eye? Any thoughts on that?

@davidlatwe
Copy link
Collaborator Author

What does it look like when a row is disabled? Or the widget is disabled?

Good question, here's my test, disabling row and widget 👇

8612a1e2-4827-48bd-a130-5bdc90e46897

It seems disabling widgets doesn't affect the highlight color, and the same result before this PR.

I always wondered whether we should make the Alternating Row color slightly subtler and easier to the eye?

I have thought about that, too. And I vote yes on that. 👍

@davidlatwe
Copy link
Collaborator Author

A quick test on dimming alternative row color (alternate-background-color)

image

From #3A3939 to #2D2C2C.

@mkolar
Copy link
Member

mkolar commented Feb 18, 2020

I like it. also with the dimming of the alternating colours. It's a bit softer which is always nice.

@davidlatwe
Copy link
Collaborator Author

Can we merge this ? 😃

@davidlatwe davidlatwe merged commit ebbbbea into getavalon:master Feb 21, 2020
@davidlatwe davidlatwe deleted the style-tweak branch February 21, 2020 07:28
davidlatwe added a commit to MoonShineVFX/avalon-core that referenced this pull request Apr 16, 2020
Style tweak, dimming highlight color
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.

5 participants