Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.0] Improve DX on the com_templates(styles) #41388

Merged
merged 15 commits into from Feb 27, 2024

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Aug 18, 2023

Pull Request for Issue #NULL .

Summary of Changes

Add a way to have the templateDetails.xml drive the entirety (basically the menu_assignment tab) of the style view.

Why?

So, right now the style edit layout has a hardcoded sub layout for the users to select the menus that the template style should be applied. If a developer/template author needs to roll their own menu selecting logic/UI/UX they have to create a plugin, observe the option/view, etc and override the layout. This is way too painful and limiting. Thus this PR, building on the fundamentals of Joomla (basically form fields) allows a way simpler way for devs to roll their own UI/UX (obviously as this is based on form fields it will require AJAX functionality but then again who's not building AJAX first UI in 2023?)

Testing Instructions

  • Apply this PR (should also work with patch testser, if patch tester is working on J5)
  • Visit administrator/index.php?option=com_templates&view=styles
  • Click on Cassiopeia - Default link (or any other style-template in the list)
  • Check the Last tab: Menu Assignment
  • Edit the Cassiopeia templateDetails.xml and after the last </fields> (line 221) add:
<fields name="assigned">
  <fieldset name="assigned" addfieldprefix="Joomla\Component\Templates\Administrator\Field">
    <field name="assigned" type="menus" />
  </fieldset>
</fields>

Also create a file administrator/components/com_templates/src/Field/MenusField.php with the following contents

content
<?php
namespace Joomla\Component\Templates\Administrator\Field;

use Joomla\CMS\Factory;
use Joomla\CMS\Form\Field\TextField;
use Joomla\CMS\Language\Text;
use Joomla\CMS\Layout\LayoutHelper;
use Joomla\Component\Menus\Administrator\Helper\MenusHelper;


// phpcs:disable PSR1.Files.SideEffects
\defined('_JEXEC') or die;
// phpcs:enable PSR1.Files.SideEffects


class MenusField extends TextField
{
    protected $type = 'Menus';

    public function setup(\SimpleXMLElement $element, $value, $group = null)
    {
        // Make sure there is a valid FormField XML element.
        if ((string) $element->getName() !== 'field'){
            return false;
        }

        // Reset the input and label values.
        $this->input = null;
        $this->label = null;

        // Set the XML element object.
        $this->element = $element;

        // Set the group of the field.
        $this->group = $group;

        $attributes = [
        'multiple', 'name', 'id', 'hint', 'class', 'description', 'labelclass', 'onchange', 'onclick', 'validate', 'pattern', 'validationtext',
        'default', 'required', 'disabled', 'readonly', 'autofocus', 'hidden', 'autocomplete', 'spellcheck', 'translateHint', 'translateLabel',
        'translate_label', 'translateDescription', 'translate_description', 'size', 'showon'];

        $this->default = isset($element['value']) ? (string) $element['value'] : $this->default;

        // Set the field default value.
        $this->value = $value;

        // Lets detect miscellaneous data attribute. For eg, data-*
        foreach ($this->element->attributes() as $key => $value) {
            if (strpos($key, 'data-') === 0) {
                // Data attribute key value pair
                $this->dataAttributes[$key] = $value;
            }
        }

        foreach ($attributes as $attributeName) {
            $this->__set($attributeName, $element[$attributeName]);
        }

        // Allow for repeatable elements
        // $repeat = (string) $element['repeat'];
        // $this->repeat = ($repeat === 'true' || $repeat === 'multiple' || (!empty($this->form->repeat) && $this->form->repeat == 1));

        // Set the visibility.
        $this->hidden = ($this->hidden || strtolower((string) $this->element['type']) === 'hidden');

        $this->parentclass = isset($this->element['parentclass']) ? (string) $this->element['parentclass'] : $this->parentclass;

        // Add required to class list if field is required.
        if ($this->required) {
            $this->class = trim($this->class . ' required');
        }

        // Hide the label
        $this->hiddenLabel = true;
        $this->hidden = true;

        return true;
    }

    public function getInput() {
        // Initialise related data.
        $menuTypes = MenusHelper::getMenuLinks();
        $user      = $this->getCurrentUser();
        $app       = Factory::getApplication();
        $currentId = $app->input->getInt('id');

        /** @var Joomla\CMS\WebAsset\WebAssetManager $wa */
        $wa = Factory::getApplication()->getDocument()->getWebAssetManager();
        $wa->useScript('com_templates.admin-template-toggle-assignment');

        $html = '<label id="jform_menuselect-lbl" for="jform_menuselect">'. Text::_('JGLOBAL_MENU_SELECTION') . '</label>'
        .  '<div class="btn-toolbar">'
        .    '<button class="btn btn-sm btn-secondary jform-rightbtn" type="button" onclick="Joomla.toggleAll()">'
        .    '<span class="icon-square" aria-hidden="true"></span> ' . Text::_('JGLOBAL_SELECTION_INVERT_ALL') . '</button>'
        .  '</div>'
        .  '<div id="menu-assignment" class="menu-assignment">'
        .    '<ul class="menu-links">';

        foreach ($menuTypes as &$type) {
            $html .= '<li>'
            . '<div class="menu-links-block">'
            .   '<button class="btn btn-sm btn-secondary jform-rightbtn mb-2" type="button" onclick=\'Joomla.toggleMenutype("' . $type->menutype . '")\'>'
            .   '<span class="icon-square" aria-hidden="true"></span> ' . Text::_('JGLOBAL_SELECTION_INVERT') . '</button>'
            .   '<h5>' . $type->title ?: $type->menutype . '</h5>';

            foreach ($type->links as $link) {
                $html .= '<label class="checkbox small" for="link' . (int) $link->value . '" >'
                . '<input type="checkbox" name="jform[assigned][]" value="'
                . (int) $link->value . '" id="link' . (int) $link->value . '"'
                . (($link->template_style_id == $currentId) ? ' checked="checked"' : '')
                . (($link->checked_out && $link->checked_out != $user->id) ? ' disabled="disabled"' : ' class="form-check-input chk-menulink menutype-' . $type->menutype . '"')
                . '/>';

                $html .= LayoutHelper::render('joomla.html.treeprefix', ['level' => $link->level]) . $link->text;
                $html .='</label>';
            }

            $html .= '</div></li>';
        }

        $html .= '</ul></div>';

        return $html;
    }
}
  • Revisit administrator/index.php?option=com_templates&view=styles
  • Click on Cassiopeia - Default link
  • Check the Last tab: Menu Assignment. Now you should have the same menus as before
  • That's that, you override the menu assignment using a custom field

Actual result BEFORE applying this Pull Request

Too much code needed for such an easy task and too complicated for template authors

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org: Template-details-file #41388 Manual#222

  • No documentation changes for manual.joomla.org needed

@HLeithner could yo have a look at this one? Thanks

@brianteeman
Copy link
Contributor

Yay - adding another option - always a popular decision.

What is the real world use case for adding this option

@dgrammatiko
Copy link
Contributor Author

Yay - adding another option - always a popular decision.

Where did you see that another option? I'm just making devs life easier: instead having to roll a plugin with a template now they could do it through the XML with a simple field.

What is the real world use case for adding this option

Obviously I have a use case thus the PR...

@richard67
Copy link
Member

As far as I can see, adding the new option happens only in the testing instructions and is only for testing.

@dgrammatiko
Copy link
Contributor Author

@SharkyKZ what's your problem and you keep downvoting my PRs? Is there something wrong code wise that you want point out or...?

@brianteeman
Copy link
Contributor

Maybe I misread the option part. I still think this needs to be justified with a real world use case. There is only one I am aware of and that was handled by the ext dev with documentation without any issue

@dgrammatiko
Copy link
Contributor Author

I still think this needs to be justified with a real world use case

Yootheme are definitely overriding the menu assignment (at least they did last time I used one of their templates). Probably also JoomShaper and many other template studio out there. This is NOT an edge case...

@brianteeman
Copy link
Contributor

You still dont explain why they are doing it. And presumably if they are then they have no problem that this pr is solving. /me confused

@dgrammatiko
Copy link
Contributor Author

@brianteeman there's a why section in the description explaining why I did this PR and what it solves (dropping a side plugin just to have this part of the UI overridden and allow the whole form/view be driven by the XML). Is there something more you want me to explain here?

@brianteeman
Copy link
Contributor

I read the why - it doesnt really explain the usecase

@dgrammatiko
Copy link
Contributor Author

Ok, let me try again.

So the route /administrator/index.php?option=com_templates&view=style&layout=edit&id=11 (where id is the style id) is just a render of the form driven from the <config> part of the templateDetails.xml of a template:

<config>
<fields name="params">
<fieldset name="advanced">
<field
name="brand"
type="radio"
label="TPL_CASSIOPEIA_BRAND_LABEL"
default="1"
layout="joomla.form.field.radio.switcher"
filter="boolean"
>
<option value="0">JNO</option>
<option value="1">JYES</option>
</field>
<field
name="logoFile"
type="media"
default=""
label="TPL_CASSIOPEIA_LOGO_LABEL"
showon="brand:1"
/>
<field
name="siteTitle"
type="text"
default=""
label="TPL_CASSIOPEIA_TITLE"
filter="string"
showon="brand:1"
/>
<field
name="siteDescription"
type="text"
default=""
label="TPL_CASSIOPEIA_TAGLINE_LABEL"
description="TPL_CASSIOPEIA_TAGLINE_DESC"
filter="string"
showon="brand:1"
/>
<field
name="useFontScheme"
type="groupedlist"
label="TPL_CASSIOPEIA_FONT_LABEL"
default="0"
>
<option value="0">JNONE</option>
<group label="TPL_CASSIOPEIA_FONT_GROUP_LOCAL">
<option value="media/templates/site/cassiopeia/css/global/fonts-local_roboto.css">Roboto (local)</option>
</group>
<group label="TPL_CASSIOPEIA_FONT_GROUP_WEB">
<option value="https://fonts.googleapis.com/css2?family=Fira+Sans:wght@100;300;400;700&amp;display=swap">Fira Sans (web)</option>
<option value="https://fonts.googleapis.com/css2?family=Noto+Sans:wght@100;300;400;700&amp;family=Roboto:wght@100;300;400;700&amp;display=swap">Roboto + Noto Sans (web)</option>
</group>
</field>
<field
name="noteFontScheme"
type="note"
description="TPL_CASSIOPEIA_FONT_NOTE_TEXT"
class="alert alert-warning"
/>
<field
name="colorName"
type="filelist"
label="TPL_CASSIOPEIA_COLOR_NAME_LABEL"
default="colors_standard"
fileFilter="^custom.+[^min]\.css$"
exclude="^colors.+"
stripext="true"
hide_none="true"
hide_default="true"
directory="media/templates/site/cassiopeia/css/global/"
validate="options"
>
<option value="colors_standard">TPL_CASSIOPEIA_COLOR_NAME_STANDARD</option>
<option value="colors_alternative">TPL_CASSIOPEIA_COLOR_NAME_ALTERNATIVE</option>
</field>
<field
name="fluidContainer"
type="radio"
layout="joomla.form.field.radio.switcher"
default="0"
label="TPL_CASSIOPEIA_FLUID_LABEL"
>
<option value="0">TPL_CASSIOPEIA_STATIC</option>
<option value="1">TPL_CASSIOPEIA_FLUID</option>
</field>
<field
name="stickyHeader"
type="radio"
label="TPL_CASSIOPEIA_STICKY_LABEL"
layout="joomla.form.field.radio.switcher"
default="0"
filter="integer"
>
<option value="0">JNO</option>
<option value="1">JYES</option>
</field>
<field
name="backTop"
type="radio"
label="TPL_CASSIOPEIA_BACKTOTOP_LABEL"
layout="joomla.form.field.radio.switcher"
default="0"
filter="integer"
>
<option value="0">JNO</option>
<option value="1">JYES</option>
</field>
</fieldset>
</fields>
</config>
with a result like:
Screenshot 2023-08-18 at 11 40 19

The only part that IS NOT driven by the XML and it is hardcoded is the menu assignment tab:

<?php if ($user->authorise('core.edit', 'com_menus') && $this->item->client_id == 0 && $this->canDo->get('core.edit.state')) : ?>
<?php echo HTMLHelper::_('uitab.addTab', 'myTab', 'assignment', Text::_('COM_TEMPLATES_MENUS_ASSIGNMENT')); ?>
<fieldset id="fieldset-assignment" class="options-form">
<legend><?php echo Text::_('COM_TEMPLATES_MENUS_ASSIGNMENT'); ?></legend>
<div>
<?php echo $this->loadTemplate('assignment'); ?>
</div>
</fieldset>
<?php echo HTMLHelper::_('uitab.endTab'); ?>
<?php endif; ?>

For a developer to roll their own UI/UX/Logic for the menu selection of a template style there is a requirement to write a plugin that will override the entirety of the layout just to have a rendering of the Menu Assignment tab.

This PR just enables the Menu Assignment tab to be rendered using the XML, in short devs don't have to roll a plugin anymore. That's all, 4 lines added all and all but an easier path to override something that is very hard to override in the current state!

@HLeithner
Copy link
Member

Basically I'm open for this change, but the way is too limited, normally we would render the complete fieldset and not a single field in a tab, like in joomla.edit.params or joomla.edit.fieldset so making this more generic would make more sense too me.

@dgrammatiko
Copy link
Contributor Author

@HLeithner code (testing instructions) updated so now the XML could have a new fields section and a fieldset named assigned. All the fields in the mentioned fieldset would be rendered at the menu assignment tab.

@richard67
Copy link
Member

Besides all I have a question regarding the title of this PR: What does “DX” stand for?

@dgrammatiko
Copy link
Contributor Author

What does “DX” stand for

Developer eXperience

@ceford
Copy link
Contributor

ceford commented Sep 12, 2023

I tested this and see the Menu Assignment panel replaced with It works Yes/No panel. But I am left wondering how I would select a menu item for this particular template style. It seems I have to write code to render and process whatever form fields I use instead of the Menu Assignment stuff. And perhaps I might add another panel to keep the Menu Assignment and do something else as well. I don't really have enough experience to say any more.


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

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:49
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@laoneo
Copy link
Member

laoneo commented Dec 15, 2023

I have tested this item ✅ successfully on 9f231fd

Tested still successfully regarding the description of the pr. I see the benefit, so it would be good if one from the template clubs can test here as well.


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

@alikon
Copy link
Contributor

alikon commented Feb 3, 2024

I have tested this item ✅ successfully on 5fd67e9


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

@Quy
Copy link
Contributor

Quy commented Feb 3, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 3, 2024
@Quy Quy removed the PR-5.0-dev label Feb 3, 2024
@Quy
Copy link
Contributor

Quy commented Feb 3, 2024

@dgrammatiko Please fix conflict. Thanks.

@dgrammatiko
Copy link
Contributor Author

Done

@dgrammatiko
Copy link
Contributor Author

@Razzo1987 can I have a decision here if this is gonna be merged for 5.1?

Thanks

@Razzo1987
Copy link
Contributor

Razzo1987 commented Feb 8, 2024

@dgrammatiko Not yet, but we will give you an update in the next few days

Thank you for your work

@Razzo1987 Razzo1987 merged commit ff55280 into joomla:5.1-dev Feb 27, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 27, 2024
@Razzo1987
Copy link
Contributor

Thank you very much @dgrammatiko !

@Quy Quy added this to the Joomla! 5.1.0 milestone Feb 27, 2024
Razzo1987 pushed a commit to joomla/Manual that referenced this pull request Feb 27, 2024
* Create template-details-file

Docs for PR joomla/joomla-cms#41388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants