? Pending

User tests: Successful: Unsuccessful:

avatar ciar4n
ciar4n
8 Aug 2017

Pull Request for Issue #16894.

Summary of Changes

This PR stops the module class suffix from been echo'd twice and moves it to the outer most module container. Moving to the outside container allows the use of grid classes (Bootstrap) in the Module Class Suffix. field. This PR only effects frontend modules. Applying the same to the backend is probably best done in a separate PR. Once done we can then remove the Bootstrap Width field which currently only works for used by the backend.

Before

Module Class Suffix = card-outline-danger

suffix

After

Module Class Suffix = card-outline-danger

suffix2

avatar joomla-cms-bot joomla-cms-bot - change - 8 Aug 2017
Category Modules Front End
avatar ciar4n ciar4n - open - 8 Aug 2017
avatar ciar4n ciar4n - change - 8 Aug 2017
Status New Pending
avatar brianteeman
brianteeman - comment - 8 Aug 2017

Once done we can then remove the Bootstrap Width field which currently only works for the backend.

It does work its just that the template doesnt have any code to support the classes added

avatar ciar4n ciar4n - change - 8 Aug 2017
The description was changed
avatar ciar4n ciar4n - edited - 8 Aug 2017
avatar ciar4n ciar4n - change - 9 Aug 2017
Labels Added: ?
avatar wilsonge
wilsonge - comment - 9 Aug 2017

OK We aren't removing the Bootstrap Width field here at all. I think standardising this in the chrome is a positive move, so merging

We should document the fact not outputting the module class suffix in the module itself is recommended :)

avatar wilsonge wilsonge - change - 9 Aug 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-08-09 14:48:33
Closed_By wilsonge
avatar wilsonge wilsonge - close - 9 Aug 2017
avatar wilsonge wilsonge - merge - 9 Aug 2017
avatar matrikular
matrikular - comment - 8 Sep 2017

I would like to point out that with this PR, one will not be able to apply a class suffix to modules that are rendered in the none system chrome. Is this the intended behavior?

avatar ciar4n
ciar4n - comment - 8 Sep 2017

True. My thinking is that 'none' should be purely just module content. Otherwise I would suggest the following be wrapped in a div to which the module class can be added.. https://github.com/joomla/joomla-cms/blob/4.0-dev/templates/system/html/modules.php#L17

avatar matrikular
matrikular - comment - 10 Sep 2017

@ciar4n thank you. My initial response included that idea as well but it feels kinda wrong to add things to the none chrome that have been in the module itself before.

While having the same class suffix in the chrome and in the module seems prone to errors, removing it from the module completly forces us to create a custom chrome and / or an override, right?

An easier solution could have been to either add a chrome class suffix field (yes, I know, we want less parameters) or a static pre-/suffix that differentiates between the chrome and the module.

Something like: badge-[chrome-sfx] or top-prize-[module-sfx] (without the brackets) would make it also easily identifiable. What do you think?

avatar BPBlueprint
BPBlueprint - comment - 6 Nov 2018

@ciar4n thank you. My initial response included that idea as well but it feels kinda wrong to add things to the none chrome that have been in the module itself before.

While having the same class suffix in the chrome and in the module seems prone to errors, removing it from the module completly forces us to create a custom chrome and / or an override, right?

An easier solution could have been to either add a chrome class suffix field (yes, I know, we want less parameters) or a static pre-/suffix that differentiates between the chrome and the module.

Something like: badge-[chrome-sfx] or top-prize-[module-sfx] (without the brackets) would make it also easily identifiable. What do you think?

I think this would be a good solution.

avatar franz-wohlkoenig franz-wohlkoenig - change - 25 Jun 2019
Title
[4.0][Frontend] Move all module class suffix to outside container
[4.0] [Frontend] Move all module class suffix to outside container
avatar franz-wohlkoenig franz-wohlkoenig - edited - 25 Jun 2019
avatar ciar4n ciar4n - change - 11 Aug 2020
The description was changed
avatar ciar4n ciar4n - edited - 11 Aug 2020

Add a Comment

Login with GitHub to post a comment