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

Theme fixes #1753

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants

arschmitz added some commits Sep 29, 2016

Theme: Switch icon background to use bgColorContent
It's more semanticly correct then fcActive and looks the same or better
on most themes

Fixes jquery/download.jqueryui.com#335
@jzaefferer

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Sep 30, 2016

Member

Diff looks good to me.

Member

jzaefferer commented Sep 30, 2016

Diff looks good to me.

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
Member

scottgonzalez commented Oct 4, 2016

@@ -119,7 +119,7 @@ a.ui-button:active,
.ui-icon-background,
.ui-state-active .ui-icon-background {
border: #003eff/*{borderColorActive}*/;
background-color: #ffffff/*{fcActive}*/;
background-color: #ffffff/*{bgColorContent}*/;

This comment has been minimized.

@scottgonzalez

scottgonzalez Oct 4, 2016

Member

Why are we pairing the content background with the active border? Why is this not using the active background?

@scottgonzalez

scottgonzalez Oct 4, 2016

Member

Why are we pairing the content background with the active border? Why is this not using the active background?

This comment has been minimized.

@arschmitz

arschmitz Oct 4, 2016

Member

The active background does not look good at all in the majority of themes. We can switch the border color to match though.

@arschmitz

arschmitz Oct 4, 2016

Member

The active background does not look good at all in the majority of themes. We can switch the border color to match though.

This comment has been minimized.

@scottgonzalez

scottgonzalez Oct 4, 2016

Member

Ok. I think it makes sense for these to match. Though it's a shame the active colors don't work here.

@scottgonzalez

scottgonzalez Oct 4, 2016

Member

Ok. I think it makes sense for these to match. Though it's a shame the active colors don't work here.

@arschmitz arschmitz closed this in 1b0e947 Oct 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment