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
Reorganized less variables for simpler theming #7951
Conversation
|
||
@theme-color-border: @color-silver-l80; | ||
|
||
@theme-color-menu-contrast-text: @theme-color-text-lighter; |
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.
@theme-color-menu-*
and @theme-color-widget-*
should be in theme.less
I think. Those are specifically exposed API's/variables to allow developers to change those. Or with other words those are the things we identified that companies might want to change if they want to adjust Piwik to their CI.
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.
OK, last time though you argued that most themes are about replacing only the main Piwik colors with the one of the company/host/agency… The menu and widget variables are linked to other variables so they should adjust when we change the base color variables (i.e. text color + background color). Anyway I don't mind moving those variables.
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.
Done
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.
Not sure where I mentioned it, eg in #7876 (comment) I mentioned to put the menu and widget colors in theme.less
. They are very important for theming. The @theme-color-background-*
variables should be still in theme-advanced
as they are "complicated" (whch I also mentioned here #7793 (comment) ).
I know they default to other variables but we want users to be able to just specifically change those widget and menu colors without affecting anything else to keep it as easy as possible. Just by changing those 5 menu and widget variables they will be able to create a nice theme according to their CI.
For me there are 2 things still unclear:
Asking 1) here as we added a couple of new variables and I would not feel good about adding more variables without having this. We can create a new issue for automatically generating it, but then we should maybe create at least a developer documentation/reference page manually. |
FYI I haven't added or removed variables in this PR, I have only reorganized them. And I believe since I started the redesign I have only added the |
The UI test fail is a random failure: http://builds-artifacts.piwik.org/ui-tests.simpler-theme/12676.7/screenshot-diffs/diffviewer.html Ready to be merged |
Feedback:
|
I have just renamed some files and moved less variables, so there should be no BC breaks. Will update the developer documentation. |
Reorganized less variables for simpler theming. Moved definition of basic LESS variables to theme.less and advanced variables to theme-advanced.less (in Morpheus).
We should still test it if possible. As mentioned earlier we had problems in the past with this just by renaming files eg because of |
See #7876 for background
We now have
theme.less
andtheme-advanced.less
to separate simple variables for plugin developers from advanced variables.In Morpheus:
That makes Morpheus have the same layout as other themes (i.e.
theme.less
contains the theme variables), which I think is great for developers that could be previously confused with the file names.By the way I took great care to make sure git moves files and not deletes/creates them, but the diff here doesn't seem to show that which is a shame (in phpstorm files are correctly moved) -_-