[#35839] - Article View: Changes to class names
- Closed
- 17 Oct 2021
- Medium
- Build: 4.0-dev
- Draft PR
- # 35839
- Diff
- impressionestudio:patch-13
User tests: Successful: Unsuccessful:
Pull Request for Issue #35342.
General description of the changes:
- New class names are added (in some cases).
- In these cases, the old and not useful class names remained for compatibility reasons.
- The old and not useful class names are separated with the special character | so the developers will distinguish them and not use them.
(The special character is allowed according to https://stackoverflow.com/questions/448981/which-characters-are-valid-in-css-class-names-selectors.)
Notes:
- Personally, I would not suggest keeping the old and not useful classes, but some guys want backward compatibility.
- I don't find BEM so useful, but since it is already used, it is better to comply.
Detailed explanation of the changes:
-
On row 36, the class "item-page" is not needed but remained for compatibility reasons, separated by the character | to be indicated as deprecated. We can use the class "com-content-article" which is more specific.
-
On row 36, the page class suffix should not be merged to any existing class name because if the suffix class is used, it cancels the usage of the existing class. If we have ".com-content-article_suffix" then we lose ".com-content-article". With a space before the suffix class we can easily use this ".com-content-article.suffix" keeping the usage of ".com-content-article".
-
On row 39, the class "page-header" is replaced by "article__title--menu" since it represents the title field of a menu item.
The old class name remained for compatibility reasons, separated by the character | to be indicated as deprecated. -
On row 53, the class "page-header" is replaced by "article__title" since it represents the title field of an article.
-
On row 101, the class "com-content-article__body" is changed to "article__content" since it represents the content tab of an article.
The old class name remained for compatibility reasons, separated by the character | to be indicated as deprecated.
Please, do not keep all this "com-content-article" to every class name. It is too big, we have it on the parent element and we don't use it on other sibling elements. So why here?
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End com_content |
| Title |
|
||||||
I dont like changing class names at this time.
I dont like the use of the |
I dont agree with most of your explanations for the name change (and if accepted would need the exact same change being made in all core components)
I dont see the benefits or why this should be done in core for everyone instead of a template override - thats what they exist for
| Labels |
Added:
?
|
||
| Category | Front End com_content | ⇒ | Front End com_content Layout |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-10-17 09:39:59 |
| Closed_By | ⇒ | impressionestudio |
I dont like changing class names at this time.
Ok, no problem, this is a personal opinion.
I dont like the use of the |
It is an unusual idea, but I find it useful. Of course, it is not necessary to follow it, or we can use it now and remove it anytime later.
I dont agree with most of your explanations for the name change (and if accepted would need the exact same change being made in all core components)
That is also personal opinion. I hope others will agree. Let's start with the content component which is the most important and later we can change all the components.
I dont see the benefits or why this should be done in core for everyone instead of a template override - thats what they exist for
I don't want to create an override for everyday actions.
Have you seen the classes "article-info text-muted" in the info_block of an article? These classes also appear on installed templates like Gantry. Why the "text-muted" class is there? I don't want it when I use another template than Cassiopeia. It is a matter of each template to give different style by using the main class and not additional specific classes (Bootstrap).
Everything you say is an explanation of why we have template overrides.
@brianteeman Could you tell me what you don't like in case I can fix it?