User tests: Successful: Unsuccessful:
Pull Request for Issue #34559.
#33100 introduced atum specific CSS variables across the core. This shouldn't be the case as extensions should be independent of the template. It would be better to have these variables with a generic name so other templates can provide them as well.
@ciar4n and @dgrammatiko any feedback?
Browse the core, especially the media manager.
All looks the same.
All looks the same.
It needs to be documented which variables a template has to provide.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration Templates (admin) NPM Change |
Please comment here @brianteeman & @dgrammatiko as this is the pr to decide about the variable setup andbia more important. #34624 is just a followup... It will be easier for others to fillow the decission path.
@wilsonge I would mark this one here as release blocker as it basically sets the path for other templates.
Sorry I saw this one second - will comment here in future
It's fine as inline styles, but this needs to be documented that AFFECTS every front end template.
Doing it in a stylesheet will mean that it can be updated in a new release without having to ask every template developer and site owner to update their own templates.
So wouldn't this be more obvious (and useful) as every template will need it
Pseudo code untested
// Enable assets
$wa->usePreset('template.cassiopeia.' . ($this->direction === 'rtl' ? 'rtl' : 'ltr'))
->useStyle('template.active.language')
->useStyle('template.user')
->useScript('template.user')
// global design classes
->useStyle('template.global');
My comment on the other PR:
It's fine as inline styles, but this needs to be documented that AFFECTS every front end template.
Also, a personal preference would be to change the prefix from template to jdt (joomla design token) or something similar. The template prefix is not really representative that these are Global design tokens. But maybe that's just me.
Doing it in a stylesheet will mean that it can be updated in a new release without having to ask every template developer and site owner to update their own templates.
You could go that way but since these should be template dependant nobody will ever use that css file. So, the best solution is documenting and communicating (eg add a multiline comment in the templates/index.php at the very top stating that COMPATIBLE JOOMLA TEMPLATES SHOULD DEFINE THESE CSS VARS )
But then if we ever need to change the vars (or add new ones) we can't
That's always the case with an API. And this is what we basically define here. During the lifecycle of version 4,we can add new ones but not rename or delete. Adding needs to come with a proper default.
But you can't add new ones your way and you can do my way
Not sure what you mean. Adding is never an issue when you define a proper default.
Let me try again.
Scenario 1 (the current proposal for inline styles)
all template builders copy the 6 lines into their own template
Joomla 4.0.1 is released with a bug fix to one of those lines (or adding a new line)
all template builders have to update their own template manually
Scenario 2 (using a template.global css file)
all template builders copy the 1 line into their own template
Joomla 4.0.1 is released with a bug fix to one of those lines (or adding a new line)
all template builders have to do nothing at all
This is only the case when the template includes the css file. But honestly I do not care if they are inline or if they are in a file. I would even prefer to have them in a file, so it would be then much clearer.
So please put them in a file and we can both be happy
How do you want to pass the colors which are coming as params into the the file then?
Sorry - I am only talking about the non-atum template which doesnt have params
@brianteeman your idea is flawed because the file can only have predefined colours which 99% will not match the template colours. So, one way or another the template builders will have to override the CSS Vars values to match their templates. Also the documentation should be a static site: design-system.joomla.org which defines the design system of Joomla sites similar to https://make.wordpress.org/design/handbook/design-guide/
These colours are only used in site admin tools like media manager so nothing to match
Think like a user not a designer or developer.
Think like a user not a designer or developer.
Users have nothing to do with this as it only affects the template builders...
cough, cough, cough, splutter, splutter.
Not every site purchases a template from a store, club or developer.
I will have a look next week and come up with a proposal.
I think it is good as it is.
But If need as a file.
Then can create one, and add global asset,
{
"name": "template.global-variables",
"type": "style",
"uri": "global-variables.css",
"description": "Joomla default CSS variables ... blabla description"
},
somewhere after here:
joomla-cms/build/media_source/system/joomla.asset.json
Lines 49 to 54 in 1b5217c
Then add it as dependency in each template.
A templates can override this file by placing it under <template>/css/global-variables.css
(or override whole asset).
However, this means that we need to update the file after User change the template parameters.
That is a hard part here :)
But for a templates that does not have such parameters it should work good, User even able to edit this file in a template editing page.
However, this means that we need to update the file after User change the template parameters.
I am only saying to move it to a file for frontend templates where there are no paramaters
That will work, I wrote it in last sentence ;)
Labels |
Added:
NPM Resource Changed
?
|
@Fedik can you help me out here with asset dependencies. Getting always the following error error on this commit:
Unsatisfied dependency "template.global-variables" for an asset "template.cassiopeia.ltr" of type "style"
looks good on quick look,
I try to test
you probably forgot to run "npm install" ,
or just copy build/media_source/system/joomla.asset.json
=> media/system/joomla.asset.json
then it will work
Thanks @Fedik!!
So what is missing here is that beside the variables from the file https://github.com/joomla/joomla-cms/pull/34621/files#diff-5261142b249534c0e5d720df7c6ba474b9d920a0329745d37fd0dfb0f1bd8405 the core needs also the bootstrap color variables from here https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/templates/atum/scss/_variables.scss#L60. This needs to either properly be documented or we have to make the file build/media_source/system/css/global-variables.css a scss and include the bS variables for a proper default. Suggestions?
OK I'll ask the dumb question :) Is there a reason these need to be in a dedicated file in the system directory and not just overridden on a template basis in user.css. If this was all our CSS vars I'd kind of get it. But we're still going to have all the boostrap vars on a template basis.
A different approach would be to give each component its own CSS variables that the template can dial into.
Example...
// build/media_source/com_media/scss/components/_media-toolbar.scss
.media-toolbar {
color: var(--com-media-toolbar-color, #333);
}
The template can then redefine the above variable, if they wish to do so, to their own template variable.
// administrator/templates/atum/scss/style.scss
:root {
--com-media-toolbar-color: var(--atum-bg-dark-60);
}
@ciar4n from separation of concern point of view this would be the right approach. But what we define here are some common variables which are used all over core to have a consistent look.
So I would rather do:
// build/media_source/com_media/scss/components/_media-toolbar.scss
.com-media {
--com-media-toolbar-color: var(--template-bg-dark-60);
}
.media-toolbar {
color: var(--com-media-toolbar-color);
}
In template:
// administrator/templates/atum/scss/style.scss
.com-media {
--com-media-toolbar-color: var(--template-bg-blue);
}
OK I'll ask the dumb question :) Is there a reason these need to be in a dedicated file in the system directory and not just overridden on a template basis in user.css. If this was all our CSS vars I'd kind of get it. But we're still going to have all the boostrap vars on a template basis.
That's my question too. How do we define all vars? What we can do is somehow include the BS vars in this file or mention in the file build/media_source/system/css/global-variables.css that the BS colors need to be included as well. I guess this is personal preference and everybody has it's own opinion. I would let the release leader decide how we are going to mention/reference the BS vars and then we can move forward as there are pros/cons for every approach.
@laoneo #34621 (comment) i like this - it seems fairly logical. although all extensions are going to be hardcoded into bootstrap anyhow for some of the sass functions.
And this is not the scope of this pr anyway. But definitely something we might consider for the future.
I don't think this dedicated SCSS file is the way to go anyhow. If we're going to allow people to override it should be in user.css i think. Can we revert that out and just rename the existing vars for now please?
So you want to have the variables inline on cassiopeia? How should the variables be renamed?
There are two different parts to this PR. The first is simply renaming variables from atum to template and I really don't have an opinion about that. The second is the creation of the build/media_source/system/css/global-variables.css
and I think its purpose is not fully understood.
There are various times that backend code is used in the frontend, such as media manager and editing an article. The problem currently is that in these cases the backend code is not styled correctly as it is expecting the value of various css variables that are not present in the frontend template. (there are numerous issues reported)
One solution (and I think this was @laoneo first proposal) would be to add each of these variables to the frontend template inline in the index.php of the template. This works but I believe it is short sighted and is where I see a problem that this PR addresses - not many people will use cassiopeia.
With the inline solution it requires every template to add these variables in the template which might not seem for an obvious reason to many people. Even if we can resolve the education issue we come to the main problem. Updating the variables or even adding new ones in future joomla releases. If we ever have to do that then every template will need to be manually updated with the new code. It cannot be automated in any way.
With the solution proposed here it only requires the template to include the global-variables. If we ever have to update, modify, add new variables etc then the site owner will gain the benefit of those changes automatically on update.
We have a similar scenario in Joomla 3 which can be seen in the core beez3 template where we load templates/system/css/system.css
I hope that helps to explain.
@wilsonge and @brianteeman I let you guys find an agreement as I don't care if they are inline or in a file. For me is important to get this pr merged as base to fix the media manager on the front end.
So I reverted as I'm tired of waiting. Lets get this one merged and then we can fix the Media manager in #34624.
@brianteeman you can do then a followup pr with the var file, which is imo still a good option.
disappointing but obviously I feel your frustration at the lack of response
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-07-15 23:12:56 |
Closed_By | ⇒ | wilsonge |
I definitely prefer @ciar4n 's proposed solution ( #34621 (comment) ) to the global variables file.
Same comment as in the other pr #34624 (comment)