? ? Success

User tests: Successful: Unsuccessful:

avatar coolcat-creations
coolcat-creations
19 Oct 2016

Summary of Changes

This PR adds a Module ID Parameter in the Advanced Tab from Modules.
This param is very useful to add IDs to module-containers in case of OnePagers/anchors or as target of a script. It improves Design-abilities so you don´t have to handle IDs inside the module-content or create overrides/Chromes for each ID.

image

Testing Instructions

Define Module ID in mod_custom or mod_newsflash and see if it works

Documentation Changes Required

  • Added Language String
  • Added Module ID output for core modules
avatar coolcat-creations coolcat-creations - open - 19 Oct 2016
avatar coolcat-creations coolcat-creations - change - 19 Oct 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2016
Labels Added: ? ?
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2016
Category Administration Components Language & Strings Front End Modules
avatar brianteeman
brianteeman - comment - 19 Oct 2016

Can we use something other than the term module id. It's going to get
confused with the ID number of the module.

On 19 Oct 2016 1:54 p.m., "coolcat-creations" notifications@github.com
wrote:

Summary of Changes

This PR adds a Module ID Parameter in the Advanced Tab from Modules.
This param is very useful to add IDs to module-containers in case of
OnePagers/anchors or as target of a script. It improves Design-abilities so
you don´t have to handle IDs inside the module-content or create
overrides/Chromes for each ID.

[image: image]
https://cloud.githubusercontent.com/assets/828371/19519306/ab9815ae-960b-11e6-9893-37771add89e7.png
Testing Instructions

Define Module ID in mod_custom or mod_newsflash and see if it works
Documentation Changes Required

  • Added Language String
  • Added Module ID output for custom and newsflash module

If PR is ok Module ID output for the other Core Modules will be added asap

You can view, comment on, or merge this pull request online at:

#12473
Commit Summary

  • Add Module ID Parameter to Advanced Options in module and modules

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#12473, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8RO5L0zawjDRygTxqCSnB2h2F3fSks5q1hMTgaJpZM4Ka7zx
.

avatar dgt41
dgt41 - comment - 19 Oct 2016

@coolcat-creations although this might be somehow useful for frontend devs, I'll say is not needed, because you already have control over the class.

So instead of jQuery('#some_id') or vanilla document.querySelector('#some_id') you can still use
jQuery('.some_id') or vanilla document.querySelector('.some_id') (as long as the class is UNIQUE in the page, otherwise you will get the first occurrence)

IMHO Joomla should go away from id's (it's an extra tag if you already have a tag class, which is true most of the times)

avatar coolcat-creations
coolcat-creations - comment - 19 Oct 2016

@dgt41 Thanks for your Feedback - For my Argument with scripting i absolutely agree, but for OnePager and "Scrolling" it´s still useful and often used these days. I understand and agree on removing params as much as possible but i think it´s good to add more power for Overrides and Chrome.

avatar coolcat-creations
coolcat-creations - comment - 19 Oct 2016

@brianteeman Thank you, that´s right. I changed to Module CSS ID ?
-> like modulecsssuffix

avatar coolcat-creations coolcat-creations - change - 19 Oct 2016
The description was changed
avatar coolcat-creations coolcat-creations - edited - 19 Oct 2016
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2016
Labels Removed: ?
avatar Bakual
Bakual - comment - 19 Oct 2016

The place where you added the field contains parameter for the module chromes, which are a template thing. To be consistent (and make sense) this would have to be supported by the template module chromes if the field is declared in this spot.

Since you do the actual support of the parameter in the module itself, the parameter should be defined in the module XML instead.

This would also reduce confusion for the user when he wants to use that new parameter with a (3rd party) module that doesn't support it.

avatar dgt41
dgt41 - comment - 19 Oct 2016

Since you do the actual support of the parameter in the module itself, the parameter should be defined in the module XML instead.

That won't work if you have the same module appearing twice in a page

avatar Bakual
Bakual - comment - 19 Oct 2016

That won't work if you have the same module appearing twice in a page

For that it doesn't matter where the field is declared.
The CSS ID has to be unique per module instance anyway. Displaying the same instance twice would create issues for sure.

avatar csthomas
csthomas - comment - 19 Oct 2016

As @Bakual mentioned there is other place where you can do that,
means by modify modChrome_... functions in joomla template.

This is not as flexible as that PR but can work for all modules by one change.
For example replace it from:

<<?php echo $moduleTag; ?> class="moduletable<?php echo htmlspecialchars($params->get('moduleclass_sfx'), ENT_COMPAT, 'UTF-8') . $moduleClass; ?>">

to:

<<?php echo $moduleTag; ?> id="mod_<?php echo $module->position, '_',  $module->id;?>" class="moduletable<?php echo htmlspecialchars($params->get('moduleclass_sfx'), ENT_COMPAT, 'UTF-8') . $moduleClass; ?>">
avatar dgt41
dgt41 - comment - 19 Oct 2016

I think the best way for this is to patch the module helper to either create on the fly the Id or create a unique class, something like position-module-name-module-id and have that also displayed in the module edit view. This way no changes in the modules are required

avatar coolcat-creations
coolcat-creations - comment - 19 Oct 2016

@dgt41 nice suggestion. i think position-id might be even enough ? What do you all think?

avatar dgt41
dgt41 - comment - 19 Oct 2016

Just patch https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/module/helper.php

and the administrator/components/com_modules/views/module/tmpl/edit.php to display a note for the id or class (whatever you choose)

avatar ciar4n
ciar4n - comment - 19 Oct 2016

@dgt41 nice suggestion. i think position-id might be even enough ? What do you all think?

Sounds good. Offers a solution for one page web sites without the need of any extra fields.

avatar dgt41
dgt41 - comment - 19 Oct 2016

And here is some code:
https://github.com/joomla/joomla-cms/blob/staging/templates/protostar/html/modules.php#L44
insert

        $moduleId      = $params->get('moduleclass_sfx') . ' ' . preg_replace('/[^A-Z0-9_\.-]/i', '', $module->position). '-' . $module->id;

and https://github.com/joomla/joomla-cms/blob/staging/templates/protostar/html/modules.php#L47
change to:

        echo '<' . $moduleTag . ' class="well ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_COMPAT, 'UTF-8') . $moduleClass . '" id="' . $moduleId . '">';

Result:
screen shot 2016-10-19 at 19 30 51

@coolcat-creations is that any good?

avatar Bakual
Bakual - comment - 19 Oct 2016

When you're going to change the module chromes to generate a CSS ID, you could as well add that parameter to the XML like done in this PR and use that value instead of calculating the id. Imho automatically creating some CSS ID isn't the best idea.
Also it would have to be done for each chrome and 3rd party templates would have to do the same.

avatar dgt41
dgt41 - comment - 19 Oct 2016

@Bakual as I wrote before the static value will not work in many cases where a module is populated more than once in a page. So the id needs to be dynamic. Think of a module that brings articles by category, someone could reuse that module in the same page to have different sections with different categories. In such case the static id will be wrong as all the instances will point to the same id. Honestly this is down to the front-end devs and doesn't even need to be part of the core (who uses protostar anyways?)

avatar Bakual
Bakual - comment - 19 Oct 2016

as I wrote before the static value will not work in many cases where a module is populated more than once in a page.

That's obvious and when it's an optional parameter it is in the responsibility of the administrator to use it right.

someone could reuse that module in the same page to have different sections with different categories

Then it is a new instance of the module with different parameters and thus also a different ID (if needed).

avatar brianteeman
brianteeman - comment - 19 Oct 2016

Some modules already have a unique Id field to address the issue of
multiple instances of the same module with different settings on the same
page.

On 19 Oct 2016 6:32 p.m., "Thomas Hunziker" notifications@github.com
wrote:

as I wrote before the static value will not work in many cases where a
module is populated more than once in a page.

That's obvious and when it's an optional parameter it is in the
responsibility of the administrator to use it right.

someone could reuse that module in the same page to have different
sections with different categories

Then it is a new instance of the module with different parameters and thus
also a different ID (if needed).


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12473 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8SEAzt5KgVBrxSUMhzJxUI3qCKLpks5q1lRJgaJpZM4Ka7zx
.

avatar coolcat-creations
coolcat-creations - comment - 24 Oct 2016

The problem is indeed that for example the menu module has it´s own id, so when i add this into Chrome i have two ids for one module. Nice solution would be maybe to have also custom fields available for modules so everyone can build the additional parameter they want to have.

avatar dgt41
dgt41 - comment - 24 Oct 2016

@coolcat-creations so with the following code:

function modChrome_id($module, &$params, &$attribs)
{
    $moduleTag     = $params->get('module_tag', 'div');
    $bootstrapSize = (int) $params->get('bootstrap_size', 0);
    $moduleClass   = $bootstrapSize != 0 ? ' span' . $bootstrapSize : '';
    $headerTag     = htmlspecialchars($params->get('header_tag', 'h3'), ENT_COMPAT, 'UTF-8');
    $headerClass   = htmlspecialchars($params->get('header_class', 'page-header'), ENT_COMPAT, 'UTF-8');
    $moduleId      = $params->get('moduleclass_sfx') . ' ' . preg_replace('/[^A-Z0-9_\.-]/i', '', $module->position). '-' . $module->id;

    if ($module->content)
    {
        echo '<' . $moduleTag . ' class="well ' . htmlspecialchars($params->get('moduleclass_sfx'), ENT_COMPAT, 'UTF-8') . $moduleClass . '" id="' . $moduleId . '">';

        if ($module->showtitle)
        {
            echo '<' . $headerTag . ' class="' . $headerClass . '">' . $module->title . '</' . $headerTag . '>';
        }

        echo $module->content;
        echo '</' . $moduleTag . '>';
    }
}

in templates/protostar/html/modules.php you can select the module style: id. This will have unique id no matter if there is an id in the module (it will be in a deeper level)

screen shot 2016-10-24 at 15 53 13

The only problem in this case is only if you don't want the module to be encapsulated in a div, then the problem with the conflicting ids needs to be solved (pre_replace ??), but is also doable. Hope it helps

avatar Bakual
Bakual - comment - 24 Oct 2016

Nice solution would be maybe to have also custom fields available for modules so everyone can build the additional parameter they want to have.

I always thought it would be nice if templates could define some module chrome parameters. That would allow a lot of nice stuff for the templates.
The problem I see is when you have multiple templates, which "chrome parameters" should be shown in the module. Most likely we would need to show them from all installed and active templates, but that would become a fun UI desaster ?

avatar coolcat-creations
coolcat-creations - comment - 24 Oct 2016

@dgt41 Yes that helped and is a great solution, but should i add this Chrome in the PR and revert the other changes or rather add your solution to the docs?

@Bakual that idea is great indeed, maybe we can find a solution how to manage this?

avatar dgt41
dgt41 - comment - 24 Oct 2016

And here is the other one that will not encapsulate the module in a div:

function modChrome_id_raw($module, &$params, &$attribs)
{
    $moduleId      = $params->get('moduleclass_sfx') . ' ' . preg_replace('/[^A-Z0-9_\.-]/i', '', $module->position). '-' . $module->id;

    echo preg_replace("#id=\".+\"#", $moduleId, $module->content);
}

This one is not tested, but it should work ?

EDIT: it needs an if statement, if id exists the replace it else append after the first space

avatar brianteeman
brianteeman - comment - 25 Oct 2016

We already display all the chromes for a module in the advanced tab

On 24 October 2016 at 14:22, Thomas Hunziker notifications@github.com
wrote:

Nice solution would be maybe to have also custom fields available for
modules so everyone can build the additional parameter they want to have.

I always thought it would be nice if templates could define some module
chrome parameters. That would allow a lot of nice stuff for the templates.
The problem I see is when you have multiple templates, which "chrome
parameters" should be shown in the module. Most likely we would need to
show them from all installed and active templates, but that would become a
fun UI desaster ?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12473 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8dspFOrFRv4tk6PScHqiqZbRumujks5q3LEZgaJpZM4Ka7zx
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

avatar coolcat-creations
coolcat-creations - comment - 27 Oct 2016

@brianteeman - this was not related to this dropdown field ;) I meant to add (more) custom parameters.

avatar brianteeman
brianteeman - comment - 14 Nov 2016

What is the status of this PR @coolcat-creations


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

avatar coolcat-creations
coolcat-creations - comment - 15 Nov 2016

i understood that it´s out of discussion having a field parameter for the id and that the user could add a chrome by himself. If i misunderstood that i´m happy to discuss further or changing the PR.

avatar roland-d
roland-d - comment - 22 Aug 2017

@coolcat-creations Is this PR still valid now that we are getting custom fields for modules?


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

avatar roland-d
roland-d - comment - 22 Aug 2017

@coolcat-creations Is this PR still valid now that we are getting custom fields for modules?


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

avatar brianteeman
brianteeman - comment - 22 Aug 2017

@roland-d thats not confirmed yet

avatar roland-d
roland-d - comment - 22 Aug 2017

@brianteeman Ok thanks.

I see there is a lot of conflicts as well that need to be resolved before we can test this.

avatar brianteeman
brianteeman - comment - 22 Aug 2017

I agree that custom fields will make this redundant so Its probably best to wait to hear on a decision about the custom fields

avatar joomla-cms-bot joomla-cms-bot - change - 22 Aug 2017
Category Administration Components Language & Strings Front End Modules Administration com_modules Language & Strings Front End com_config Modules Components
avatar franz-wohlkoenig franz-wohlkoenig - change - 22 Aug 2017
The description was changed
Status Pending Information Required
avatar joomla-cms-bot joomla-cms-bot - edited - 22 Aug 2017
avatar roland-d
roland-d - comment - 23 Aug 2017

Referencing #17490 (Custom Fields for Modules)

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Sep 2017

@coolcat-creations can this be closed in favor of #17490?

avatar coolcat-creations
coolcat-creations - comment - 17 Sep 2017
avatar coolcat-creations coolcat-creations - change - 17 Sep 2017
Status Information Required Closed
Closed_Date 0000-00-00 00:00:00 2017-09-17 10:56:34
Closed_By coolcat-creations
Labels Added: ?
avatar coolcat-creations coolcat-creations - close - 17 Sep 2017

Add a Comment

Login with GitHub to post a comment