Skip to content

[4.0] Change 'Module Class Suffix' to 'Module Class'#17023

Closed
ciar4n wants to merge 3 commits intojoomla:4.0-devfrom
ciar4n:mod-class
Closed

[4.0] Change 'Module Class Suffix' to 'Module Class'#17023
ciar4n wants to merge 3 commits intojoomla:4.0-devfrom
ciar4n:mod-class

Conversation

@ciar4n
Copy link
Contributor

@ciar4n ciar4n commented Jul 9, 2017

Pull Request for Issue #16894 .

Summary of Changes

Changes 'Module Class Suffix' to 'Module Class'.

Testing Instructions

Add class to module using the 'Module Class' field (no space). Check frontend and ensure class has been added correctly.

Eg.
<div class="card yourClass">

Expected result

Actual result

Documentation Changes Required

Yes

@Bakual
Copy link
Contributor

Bakual commented Jul 9, 2017

I foresee someone complaining about a useless space in the module class in case no class is specified in the parameter.

@mbabker
Copy link
Contributor

mbabker commented Jul 9, 2017

HTML is for the browser, not the user, to read and process. The only people that are going to groan about it are the ones trying to optimize the HTML response anyway and they're going to do things like enable compression and minifiers anyway.

@Bakual
Copy link
Contributor

Bakual commented Jul 9, 2017

I think we merged PRs to remove such spaces in the past, that's why I said it. Personally I don't care at all about that space.

COM_MODULES_FIELD_MODULE_LABEL="Module Type"
COM_MODULES_FIELD_MODULECLASS_SFX_DESC="A suffix to be applied to the CSS class of the module. This allows for individual module styling."
COM_MODULES_FIELD_MODULECLASS_SFX_LABEL="Module Class Suffix"
COM_MODULES_FIELD_MODULECLASS_SFX_DESC="An additional CSS class added to the module. This allows for individual module styling."
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it is better to say

COM_MODULES_FIELD_MODULECLASS_SFX_DESC="Additional CSS class to be applied to the module. This allows for individual module styling."
 
As it can be mutliple classes. Also shouldnt we take the opportunity to rename the strings to just
COM_MODULES_FIELD_MODULECLASS_DESC
and
COM_MODULES_FIELD_MODULECLASS_LABEL

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. Renamed as suggested.

@brianteeman brianteeman modified the milestone: Joomla 4.0 Jul 19, 2017
@davidsteltz
Copy link

I have tested this item 🔴 unsuccessfully on 1ccafec

The class was added, but the backend label is broken.label


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

@davidsteltz
Copy link

Also just realized the space is NOT being added automatically.
space


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

@N6REJ
Copy link
Contributor

N6REJ commented Aug 27, 2017

I thought about fixing this, but then I noticed that "moduleclass_sfx" is a field in the database, so if that is changed for the installers, then when that person updates it won't be changed so the existing sfx will be lost for ALL modules!!!! that doesn't sound very smart.

Simply changing echo $params->get('moduleclass_sfx') to echo " " . trim($params->get('moduleclass_sfx'))
& echo $moduleclass_sfx; to echo " " . trim($moduleclass_sfx); should fix the space problem

@N6REJ
Copy link
Contributor

N6REJ commented Aug 28, 2017

infact, the more I think about this the more I think its a horrible idea... we just had a pr to remove the " " and now we're putting it back? It is a suffix so I'm not sure I see the valid reason to change it. We've done a lot of just changing stuff cause, I'm not sure we want to continue that trend.

@ciar4n
Copy link
Contributor Author

ciar4n commented Aug 28, 2017

@N6REJ PR to remove the " "? This class suffix field has been in Joomla for years. With modern CSS architecture, a suffix on a block level class is a ridiculous idea and you will be hard pushed to find anyone actually using it in that manner. And frankly if they are, you would seriously have to question the code.

Closing as to many conflicts after #17447, currently effects user data and..

We've done a lot of just changing stuff cause, I'm not sure we want to continue that trend.

@ciar4n ciar4n closed this Aug 28, 2017
@ciar4n ciar4n deleted the mod-class branch October 9, 2017 11:10
@ciar4n ciar4n restored the mod-class branch October 9, 2017 11:10
@zero-24 zero-24 removed this from the Joomla 4.0 milestone Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants