-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[4.0] HTML class naming standard [frontend][com-tags] #20819
Conversation
<?php endif; ?> | ||
<div class="com-tags__items"> | ||
<?php if ($this->params->get('filter_field') || $this->params->get('show_pagination_limit')) : ?> | ||
<form action="<?php echo htmlspecialchars(JUri::getInstance()->toString()); ?>" method="post" name="adminForm" id="adminForm" class="com-tags__items"> |
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.
com-tags__items
again? you added a div
before with this class
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.
Thank you. Well spotted. Inside com-tags__items
removed.
@@ -16,7 +16,7 @@ | |||
|
|||
?> | |||
|
|||
<div class="tag-category"> | |||
<div class="com-tags-tag-list tag-category"> | |||
|
|||
<?php if ($this->params->get('show_page_heading')) : ?> | |||
<h1> |
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.
com-tags-tag-list__heading
?
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.
Till now I haven't been adding classes to the headings. That is not to say that I shouldn't. My reasons were simply that they are generally select-able via the H1/H2 tag. It was previously suggested that classes added be kept to a minimum (#20147).
Also there seems to be some variation between calling them 'titles' and 'headings' with each view.
I am happy to add a class to headings/titles if recommended.
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.
Thank, i miss that talk. Now I have a better background, of what you try it.
I have tested this item ✅ successfully on 44ad035 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20819. |
1 similar comment
I have tested this item ✅ successfully on 44ad035 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20819. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20819. |
Pull Request for Issue #15279 .
Summary of Changes
Adds HTML classes using the following standard to com-tags frontend views.
ExtensionType-Extension(-SubExtension)(-View)
(Eg.mod-tags-tag-list
).Note: If the default view then
-View
is omitted. If only one SubExtension exists or if the SubExtension matches the Extension then-SubExtension
is omitted.Child views and child elements within the views
BEM class naming is adopted..
ExtensionType-Extension(-View)
ExtensionType-Extension__Element
.For B/C, old classes remain.
Testing Instructions
Code review.
Expected result
All works fine
Actual result
All works fine
Documentation Changes Required
Yes