? ? Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
4 Jun 2022

Summary of Changes

It redo of #32980, but without use of ID attribute

Testing Instructions

Select background Image for mod_custom,
And check module on the site, also the module HTML markup

Actual result BEFORE applying this Pull Request

Background visible, module have an ID attribute

Expected result AFTER applying this Pull Request

Background visible, module do not have an ID attribute

Documentation Changes Required

none

avatar Fedik Fedik - open - 4 Jun 2022
avatar Fedik Fedik - change - 4 Jun 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jun 2022
Category Modules Front End Templates (site)
avatar Fedik Fedik - change - 4 Jun 2022
Title
[4.1] Remove ID attribute from mod_custom
[4.2] Remove ID attribute from mod_custom
avatar Fedik Fedik - edited - 4 Jun 2022
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jun 2022
Category Modules Front End Templates (site) Administration com_installer Modules Front End Templates (site)
avatar brianteeman
brianteeman - comment - 11 Jun 2022

The changes to administrator/components/com_installer/src/Controller/InstallController.php look unrelated to me

avatar Fedik Fedik - change - 12 Jun 2022
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 12 Jun 2022
Category Modules Front End Templates (site) Administration com_installer Modules Front End Templates (site)
avatar Fedik
Fedik - comment - 12 Jun 2022

That because it was for 4.1 first.
Should be good now.

avatar simbus82
simbus82 - comment - 15 Jun 2022

Why the ID needs to be removed?

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

This pull requests has been automatically converted to the PSR-12 coding standard.

avatar HLeithner HLeithner - change - 27 Jun 2022
Labels Added: ?
avatar chmst
chmst - comment - 28 Jun 2022

@simbus82 This is why: #37873 (comment) - multiple IDs are possible.

@Fedik
Do we need an extra classname? Other modules don't.
The variablename $modid sounds like and ID but makes a classname.

avatar Fedik
Fedik - comment - 28 Jun 2022

Do we need an extra classname? Other modules don't.

The code changes use of ID to class-[id]

avatar simbus82
simbus82 - comment - 28 Jun 2022

@simbus82 This is why: #37873 (comment) - multiple IDs are possible.

Ok, it's clear that we can't have multiple ID in a single DOM (but only for validation): in 2022 is only a problem if we try to pointing them wrongly with CSS and JS.
Removing an ID (in the core since one year) can break lot's of templates that rely on this method to apply some css styles.
I hope users will be informed about this B/C.

avatar brianteeman
brianteeman - comment - 2 Jul 2022

As pointed out before by others removing the id will be an isssue for any site that is using the id for css and/or js (I am) and while it can be argued that this is not covered by the b/c promise is it really necessary?

The only way that you can have one module (with the same id) published more than once is to additionally load the module in some content with the loadmodule plugin. As that is a deliberate action by the web developer to publish the module twice on the same page when it makes no sense to do so I would expect the occurance of this to be infinitely less than the use of the id or js/css

avatar Fedik
Fedik - comment - 2 Jul 2022

I just sugested a solution to avoid duplicated ids (even if it rare possible) :)
It can be accepted or closed, I am fine with both.

avatar HLeithner
HLeithner - comment - 2 Jul 2022

Mod_custom is the module used as reference for new module that should not suggest to use wrong syntax.

avatar Fedik
Fedik - comment - 2 Jul 2022

For keep b/c I can revert ID (for people who use js), but the styling (background image) will be done with class. And will remove ID in j5 (if no one forget).
Deal? :)

avatar Mrsuperstar304
Mrsuperstar304 - comment - 7 Jul 2022

Hey everyone, I am new here. Please tell me how to start here and what are the skills required to contribute here.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37976.

avatar chmst
chmst - comment - 7 Jul 2022

@Mrsuperstar304 thank you for your interest in contributing to Joomla. Maybe you start with this video https://www.youtube.com/watch?v=30JYBOeNVTA. Or search for "how to contribute " in joomla documentation.

avatar simbus82
simbus82 - comment - 7 Jul 2022

Mod_custom is the module used as reference for new module that should not suggest to use wrong syntax.

And it's a wrong syntax to not have an unique ID for every instance of rendered mod_custom in the DOM ?

avatar Fedik
Fedik - comment - 10 Jul 2022

I have restored ID, and added a comment that it should be removed in next Major version

avatar Fedik Fedik - change - 22 Mar 2023
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2023-03-22 10:46:20
Closed_By Fedik
Labels Added: ?
Removed: ?
avatar Fedik Fedik - close - 22 Mar 2023

Add a Comment

Login with GitHub to post a comment