-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
XWIKI-13912: Impossible to control contrast of the panel headers on Flamingo #2497
Conversation
…lamingo * Added the panel header text color to the ThemeSheet.xml * Changed the name of the variable to fit other variables in the tab
…lamingo * Updated color themes
…lamingo * Fixed property name
…lamingo * Fixed colortheme files and homogeneized property names
…lamingo * Fixed codestyle * Fixed consistency
@xwiki-panel-header-bg: $theme.panelHeaderBackgroundColor; | ||
@xwiki-panel-header-text: $theme.panelHeaderTextColor; | ||
@panel-header-bg: $theme.panelHeaderBackgroundColor; | ||
@panel-header-text: $theme.panelHeaderTextColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to (additionally) keep the old variable names for backwards-compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 5587492 👍
@xwiki-panel-header-bg: $theme.panelHeaderBackgroundColor; | ||
@xwiki-panel-header-text: $theme.panelHeaderTextColor; | ||
@panel-header-bg: $theme.panelHeaderBackgroundColor; | ||
@panel-header-text: $theme.panelHeaderTextColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, wouldn't it be better to (additionally) keep the old variable names for backwards-compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 5587492 👍
…lamingo * Fixed backward compatibility
@panel-header-bg: $theme.panelHeaderBackgroundColor; | ||
@panel-header-text: $theme.panelHeaderTextColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong less variable name, this should be @xwiki-panel-header-bg
/-text
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I did not check my changes properly before commiting. Fixed in dc4c9d5
Yes, it was a bit lengthy ^^' |
…lamingo * Fixed backward compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, but there is one theme file that looks wrong, see my comment.
@@ -2050,6 +2063,9 @@ a.list-group-item { | |||
<property> | |||
<panel-bg>#060606</panel-bg> | |||
</property> | |||
<property> | |||
<panel-default-text>#ffffff</panel-default-text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a bad merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 9c640c2 👍
…lamingo * Fixed bad merge
@@ -2125,7 +2138,7 @@ div.main { | |||
box-shadow: none; | |||
} | |||
|
|||
@xwiki-panel-header-text: @navbar-default-color; | |||
@panel-header-text: @navbar-default-color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this either a) set the corresponding theme variable or b) also set the fallback value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better backwards compatibility, and consistency with the choice we made in variablesInit.vm
, I added back the former value as a fallback in afc21a6 👍
…lamingo * Fixed backward compatibility
…lamingo (#2497) * Added the panel header text color to the ThemeSheet.xml * Changed the name of the variable to fit other variables in the tab * Updated color themes * Fixed property name * Fixed colortheme files and homogeneized property names * Fixed codestyle * Fixed consistency * Fixed backward compatibility
…lamingo (xwiki#2497) * Added the panel header text color to the ThemeSheet.xml * Changed the name of the variable to fit other variables in the tab * Updated color themes * Fixed property name * Fixed colortheme files and homogeneized property names * Fixed codestyle * Fixed consistency * Fixed backward compatibility
Jira
https://jira.xwiki.org/browse/XWIKI-13912
PR Changes