User tests: Successful: Unsuccessful:
Pull Request for Issue #15279 .
Applies BEM class naming to top level elements in the login module. Initial PR with the intent to apply (if favorable) similar PRs to modules, components and layouts.
For these high level 'blocks' (https://en.bem.info/methodology/quick-start/) the intent is to use the Joomla architecture...
ExtensionType-Extension-View
(Eg. com-content-archive
).
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End |
Reference URL for BEM HTML class naming.
http://getbem.com/naming/
@franz-wohlkoenig yes. If this one gets merged, then we know that the core will move to BEM.
@brianteeman yes, the element has two underscores and the modifier one, https://en.bem.info/methodology/naming-convention/.
Labels |
Added:
?
|
What we do need to think about as well is the folder structure for the sass files of the blocks for easy reusability.
What we do need to think about as well is the folder structure for the sass files of the blocks for easy reusability.
Note: I suspect I need to stress that these particular classes are largely for identification rather than reusable OOCSS classes. That is why in BEM terms you will mostly see Blocks and Elements but no Modifiers in the changes. As we are using Bootstrap there is little or no need to be adding our own reusable modifiers. There may be cases where will will add modifier classes but I think best to add where needed rather than as a rule.
I am only focusing on top level blocks (eg. https://github.com/joomla/joomla-cms/blob/4.0-dev/modules/mod_login/tmpl/default.php#L27) and their subsequent children which I believe will cover the majority of identification required.
For block naming I was thinking along the lines of... ExtensionType-Extension-View
. For default.php the view is left out. Template files (loadTemplate) I would consider Elements. Maybe someone has a better suggestion or can improve on this one?
mod-login
- /modules/mod_login/tmpl/default.php
mod-login-logout
- /modules/mod_login/tmpl/default_logout.php
com-content-article
- /components/com_content/tmpl/article/default.php
com-content-article__links
- /components/com_content/tmpl/article/default_links.php
com-content-category
- /components/com_content/tmpl/category/default.php
com-content-category-blog
- /components/com_content/tmpl/category/blog.php
joomla-pagination-links
- /layouts/joomla/pagination/links.php
joomla-content-tags
- /layouts/joomla/content/tags.php
A strict naming convention for HTML classes adoptable throughout Joomla.
All elements are uniquely identifiable and can be selected within CSS. This excludes framework classes (Bootstrap). This does not mean that every element needs to have class. In most cases elements can be selected by the class of a parent element.
The root element (containing div) of a view can be easily identified within the DOM.
By looking at the site source one can easily identify where an element is been rendered from within the Joomla architecture. Eg. It is easy to see from the following screenshot where the pagination rendered from (https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/pagination/list.php) ...
Would that have some influence on the custom elements?
Would that have some influence on the custom elements?
@dgrammatiko ??
@laoneo @ciar4n well no, or to be more precise here it shouldn't conflict. But let me elaborate a little bit here explaining some basic concepts.
So for most of our components we will use just custom elements as we do want them to share the same css as the rest of the page (also the way we use forms prohibit us from using webcomponents for form elements or eg tabs that might contain form inputs). So whatever will be decided here custom elements will naturally support it and embrace it.!
PS For the few true web components BEM really makes 0 sense as any of the old class way makes very little sense to styling: eg: in the next image the styling is done in the <img>
element itself and in the ::root which in this case is ttc-lazy-img
(no need for classes, id's etc)
If this one gets merged, then we know that the core will move to BEM.
Who decides if this pr gets merged? I hope that we need more than just 2 successful tests for it.
mod-login-logout__login-greeting
Are you joking?
-1 for this pr. Who needs it? Just 5000 more BC breaks while upgrading from J3 to J4
i know that markup is not part of the b/c policy but we really should not be changing this just because. There has to be a good reason/benefit before this sort of change
my 5c (inflation and tax)
Just 5000 more BC breaks while upgrading from J3 to J4
You realise that you already have these breaks just by updating to BS4, right?
read it again Dmitris. these are joomla class names not bootstrap class names.
Just because semver says you can break things doesnt mean you should do without a very good reason. And I am asking what is that very good reason before I make up my own mind if this is a positive change or not.
So this is meant as a general comment as a whole and not directed toward any one person's work or effort.
Right now contributing to core and heavily the 4.0 work we mostly have a contributor pool who came onboard after 3.0's release in 2012 (some of you were probably using Joomla from 2.5 and before but weren't actively engaged here). Myself and Brian are struggling to count this, but aside from us two, @infograf768, and a couple others who've been longer term "come and go" contributors/maintainers, there aren't many involved in the project's teams right now who have also lead/managed work on a major version release. I point this fact out because while all of us would love to modernize all the things and make all the changes we would love to see happen and be able to use the "SemVer says we can break compatibility" excuse to justify it, the fact is Joomla is a distributed application with a vast user base and that means that those compatibility breaks MUST be well justified and documented if accepted (essentially as it relates to B/C you almost have to default to keeping the status quo and have a well constructed statement supporting changing it, that statement is going in part to be what convinces the code maintainers/owners the break is worth it and the communication end users see in why the break was made and how it is mitigated).
We've made mistakes in how our major version releases were put out in the past. We've learned lessons from that, either as end users consuming the software or the ones involved in the project working on those efforts. Use that knowledge and experience wisely when decision making.
And I am asking what is that very good reason before I make up my own mind if this is a positive change or not.
Well I am not the one to actually answer this, but to me this seems another valid proposal from @ciar4n that will eventually help the project:
Sort term effect is the break of old CORE ONLY layouts
Long term effect (if this is followed correctly) a better css codebase.
But to put things in perspective here I'm neutral on this. I'm not a css expert so my point of view is just my point of view. But when someone who's actually expert in a field makes an effort to introduce another way before shutting them down we should very carefully listen and evaluate the actual proposal.
Also here I just provided the info that I've being asked (for CE) and made the obvious comment that layouts are already broken...
But when someone who's actually expert in a field makes an effort to introduce another way before shutting them down we should very carefully listen and evaluate the actual proposal.
Definitely not trying to shut it down. I think a lot of us "core contributors" grasp why the proposal exists and understand its benefits. But it's not just about ensuring you, or I, or any of the few dozen active folks on the project teams understand the logic behind it. It's also about ensuring that we can clearly communicate to end users why the change is being made; it's in part ensuring we don't make changes for the sake of making changes (just because we can doesn't mean we should) but making sound changes with a clear benefit.
mod-login-logout__login-greeting
Are you joking?
If we shy away from code based purely on its looks then I think we're often missing the point. 3 of the first 4 big brand sites I have just checked are all using BEM (http://www.bbc.com/, https://www.cnn.com & https://www.nationalgeographic.com/).
Just 5000 more BC breaks while upgrading from J3 to J4
I am failing to think of one? Maybe template providers targeting existing J3 classes in their CSS. It is hardly that big of a break and even they would appreciate a definite class naming convention. Currently you could expect anything. Take element root class names for example. Currently you could easily find any of the following...
Maybe template providers targeting existing J3 classes in their CSS.
Exactly the point
It is hardly that big of a break and even they would appreciate a definite class naming convention
If they have to change all their templates for no gain then ...
And its easy for them but is it easy for the non specialist user
So on this PR's specific changes, out of context it is a minor B/C break by changing class names. No argument there.
To me, looking at things in the bigger picture, this would be my decision making process:
Is the HTML markup structure for the module still close enough to the existing Joomla 3 markup structure that existing CSS rules could still work without tweaks? If so, I'd suggest keeping the old class name as well as adding the BEM class name and adding deprecation comments (which we do have elsewhere in core, these are probably the hardest deprecations to track though).
Is the HTML markup structure for the module so vastly different that existing CSS rules wouldn't apply in any sane way? If so, then accepting this break on its own context doesn't really make things easier or harder and I'd say merge as is.
Is the HTML markup structure for the module still close enough to the existing Joomla 3 markup structure that existing CSS rules could still work without tweaks
In this case it is hard to say as the root element did not have a class. The structure has changed as one would expect with a BS2 to BS4 conversion but excluding that, not hugely. As the module is largely bootstrap elements, I suspect any use of the few non bootstrap classes would have been to target the inside bootstrap classes which will make such CSS now redundant anyways.
Mabe leave the old classes there and add the BEM ones? And with 5.0 clsan it up? No B/C issues this way.
You still have the ugly BEM classes of course.
Ok. I've added back in the old classes. I'm still of the opinion in most cases removing these classes would have minimal b/c effect. There is some obvious classes that would need to stay but I am failing to notice any in this PR. Certainly other changes have been made to the views that would cause similar minor b/c breaks and never seemed to be a problem.
If so, I'd suggest keeping the old class name as well as adding the BEM class name and adding deprecation comments (which we do have elsewhere in core, these are probably the hardest deprecations to track though).
How best to add deprecation comments? The only neutral way that I know of to mark actual classes visually is wrap them in square brackets...
<div class="mod-login__pretext [ pretext ]">
I don't mean to get peoples hair up here. I'm just looking to add a coding standard in an area where Joomla completely lacks one. BEM is currently the most common standard in this area. I have considered others but they either clash with their terminology (using words like component, module etc.) or would not cover all eventualities within Joomla. Yes it may be considered ugly and people suggesting this is a joke but I am not hearing any other suggestions.
Switching to BEM is all well and good, as it's a popular standard, however I'd suggest only doing it for classes that are being used. There's absolutely no point in keeping classes in views, if they're not being targeted via CSS or JS.
That said, I would say that adding the like of mod-login
to the module wrapper element would be a nice "to have" so users can perform basic styling changes without having to create a template override.
So to sum it up:
Good:
<div class="mod-login">
<input type="text" name="username">
<input type="password" name="password">
</div>
Bad:
<div class="mod-login">
<input type="text" name="username" class="mod-login__username">
<input type="password" name="password" class="mod-login__password">
</div>
Same applies for other core extensions. Only use the likes of class="mod-login__password"
if we're actually targeting it.
Switching to BEM is all well and good, as it's a popular standard, however I'd suggest only doing it for classes that are being used. There's absolutely no point in keeping classes in views, if they're not being targeted via CSS or JS.
I do agree however the issue here is how do we decide what classes would be used and what would not? Seems more logically to add them as a rule rather than having to ask that question with every class.
Also suggesting we only add classes to top level divs and their subsequent children (if they exist). Not each element...
<div class="mod-login">
<div class="mod-login__form">
<input type="text" name="username">
<input type="password" name="password">
</div>
<div class="mod-login__whatever">
<ul>
<li>whatever</li>
<li>whatever</li>
<li>whatever</li>
</ul>
</div>
</div>
If you add them willy nilly, you're going to have a bloated DOM and a lot of unused classes. Remember that CSS is there for styling, not to be used an semantic identifiers.
Joomla core, is supposed to provide a core base for users to build on top of, allowing them to then extend it in any way they wish.
IMO it would be best to just stick to top level classes and of course continue to use the likes of mod-login__password
, but only when being used. My 2 pence.
If you add them willy nilly, you're going to have a bloated DOM and a lot of unused classes.
I am not suggesting we do.
IMO it would be best to just stick to top level classes and of course continue to use the likes of mod-login__password, but only when being used.
How do you deem a class as been 'used'?
<div class="foo"></div>
Used 1:
.foo {
background: #000;
}
Used 2:
var foo = document.querySelector('.foo')
Really? You are missing my point. Any class we add will potentially be 'used'. Add a class anywhere and someone somewhere is gonna use it. The whole point here is to do it in a way that has some kind of consistent reasoning. Just because we don't use a class with the default template doesn't mean there isn't good reason to have one.
Potentially, meaning by a user, or in core?
User
An example from real Joomla life:
In the last years many redundant blocks have been moved into JLayouts. What will happen with these rigid BEM classes in the future then. Changing it from mod-idontknow__whatever
to joomla-forms-idontknow__whatever
? Or has a pr like this to wait until Joomla 7 before it can be merged? Or an inconsistent mixture ? Or frustrating users that have to change these classes in their CSS files? Or?
At the end of the day BEM is unflexible in a flexible system like Joomla
Don't get me wrong, I support the BEM approach, but adding classes for potential usage just reminds me of: #10011 when it should be up to the user if they want those classes or not.
Obviously I don't agree. For me it is overkill to have to create an override just to add a class. I am not even suggesting we swarm the views with classes. Just enough that all elements are easily selectable and in a manner that follows some kind of standard.
@ReLater we have the same problem too without BEM. When we move some code into a layout which has some extension specific CSS classes then we do not want them in the layout. The idea is to start working in blocks, like (the class names of the blocks are just an example and hacked quickly together):
<div class="mod-login">
<div class="mod-login__form">
<div class="joomla-intro"><?php echo JText::_('INTRO'); ?></div>
<div class="joomla-inputs joomla-inputs_horizontal">
<input type="text" name="username">
<input type="password" name="password">
</div>
</div>
</div>
Using a concept like BEM actually forces the developer to think about the markup upfront.
It would be nice to get a decision on this due to its knock-on effect to any subsequent changes to the views?
I made very good experiences with BEM and on a long term it would be good for the devs to have a strategy for class naming.
I'm supporting this as well
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-06-04 21:31:44 |
Closed_By | ⇒ | wilsonge |
It would be great if there was some documentation published on the docs site for this to aid developers
does bem require the double underscore?