Skip to content

[4.0] [Frontend] Move all module class suffix to outside container#17447

Merged
wilsonge merged 2 commits intojoomla:4.0-devfrom
ciar4n:class-suffix
Aug 9, 2017
Merged

[4.0] [Frontend] Move all module class suffix to outside container#17447
wilsonge merged 2 commits intojoomla:4.0-devfrom
ciar4n:class-suffix

Conversation

@ciar4n
Copy link
Contributor

@ciar4n ciar4n commented Aug 8, 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

@brianteeman
Copy link
Contributor

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


$params->def('count', 10);
$moduleclass_sfx = htmlspecialchars($params->get('moduleclass_sfx'), ENT_COMPAT, 'UTF-8');
$list = ArticlesArchiveHelper::getList($params);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

scrolling="<?php echo $scroll; ?>"
frameborder="<?php echo $frameborder; ?>"
class="wrapper<?php echo $moduleclass_sfx; ?>" >
class="wrapper" >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove space before >

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @C-Lodder

@wilsonge
Copy link
Contributor

wilsonge commented Aug 9, 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 :)

@wilsonge wilsonge merged commit 0ae814d into joomla:4.0-dev Aug 9, 2017
@wilsonge wilsonge added this to the Joomla 4.0 milestone Aug 9, 2017
@ciar4n ciar4n deleted the class-suffix branch August 9, 2017 15:03
@matrikular
Copy link
Contributor

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?

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 8, 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

@matrikular
Copy link
Contributor

@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?

@BPBlueprint
Copy link

@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.

@ghost ghost changed the title [4.0][Frontend] Move all module class suffix to outside container [4.0] [Frontend] Move all module class suffix to outside container Jun 25, 2019
@drmenzelit drmenzelit mentioned this pull request Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants