joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.1] Adding notice to global configuration for additional options in SEF plugin

Open LadySolveig opened this issue 1 year ago • 17 comments
trafficstars

Pull Request for PR #42692 #42702 #42704 .

Summary of Changes

Add note to gloabl configuration for additional options in system plugin SEF

grafik

Testing Instructions

  • Apply patch
  • open global configuration /administrator/index.php?option=com_config

Actual result BEFORE applying this Pull Request

no notice shown

Expected result AFTER applying this Pull Request

notice shown in section/fieldset "SEO"

Link to documentations

Please select:

  • [ ] Documentation link for docs.joomla.org:

  • [X] No documentation changes for docs.joomla.org needed

  • [ ] Pull Request link for manual.joomla.org:

  • [X] No documentation changes for manual.joomla.org needed

LadySolveig avatar Feb 18 '24 16:02 LadySolveig

I have tested this item :white_check_mark: successfully on e42c9d5e64b3f1379fac8067e9c27bddc440211e


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

Hackwar avatar Feb 19 '24 09:02 Hackwar

Sorry, this looks wrong, the layout already have "description" rendering, https://github.com/joomla/joomla-cms/blob/e42c9d5e64b3f1379fac8067e9c27bddc440211e/layouts/joomla/content/options_default.php#L26-L28

Need to look why it does not work as expected.

Fedik avatar Feb 19 '24 10:02 Fedik

No, this is a different description. $displayData->description is not filled from the XML. To be honest, the old code looks like it was never tested.

Hackwar avatar Feb 19 '24 10:02 Hackwar

Then we have to fix that, instead of hardcoding.

From what I see the description already expected from $displayData->description in same way as label done with $displayData->name

Fedik avatar Feb 19 '24 10:02 Fedik

The fix is this PR here. $displayData->description is simply wrong.

Hackwar avatar Feb 19 '24 10:02 Hackwar

Why?

Fedik avatar Feb 19 '24 10:02 Fedik

The description need to be added here https://github.com/joomla/joomla-cms/blob/e7f5cc182bb1cd39c7f085d529888ac4fa6fc77e/administrator/components/com_config/tmpl/application/default_seo.php#L16 Beside $this->name:

$this->name = 'blabla name'
$this->description = 'blabla description';

But, this also means that every other layouts in ../tmpl/application should have "description":

$this->name = 'blabla name'
$this->description = '';

Otherwise, the last added description will be rendered in all following layouts.

Fedik avatar Feb 19 '24 10:02 Fedik

Yes, and that is why this shouldn't be done like that. The whole layout obviously has only been created for the global configuration and has not been maintained properly so far. For example, the whole rendering of fields can be replaced with echo $displayData->form->renderFieldset($fieldname);.

Hackwar avatar Feb 19 '24 10:02 Hackwar

Yes, and that is why this shouldn't be done like that. The whole layout obviously has only been created for the global configuration and has not been maintained properly so far.

Sorry, I cannot follow your logic. We better stick to what we have here instead of hardcoding here and there.

If someone want to rewrite whole rendering, well, I do not mind. But then it should be done for whole view, not only here.

Fedik avatar Feb 19 '24 10:02 Fedik

@Fedik https://github.com/joomla/joomla-cms/pull/42692#issuecomment-1945895937 I have now simply taken up this approach for a different proposal. https://github.com/joomla/joomla-cms/blob/0aba9c89701154c215d3d26f2597396e4c1e9ee2/administrator/components/com_config/tmpl/component/default.php#L101

LadySolveig avatar Feb 19 '24 10:02 LadySolveig

I understood that, but as you can see application view rendred diferently from component view. I do not know why, but unless someone want to rewrite it, it is better to follow of what it is.

To have a better styling can probably change https://github.com/joomla/joomla-cms/blob/e42c9d5e64b3f1379fac8067e9c27bddc440211e/layouts/joomla/content/options_default.php#L26-L28

to:

<?php if (!empty($displayData->description)) : ?>
  <div class="alert alert-info">
    <span class="icon-info-circle" aria-hidden="true"></span><span class="visually-hidden"><?php echo Text::_('INFO'); ?></span>
    <?php echo $displayData->description; ?>
  </div>
<?php endif; ?>

Similar to what you made, but with use of 'expected' description property.

Fedik avatar Feb 19 '24 11:02 Fedik

@LadySolveig looks good now, thanks!

Fedik avatar Feb 19 '24 16:02 Fedik

I have tested this item :white_check_mark: successfully on e8f9963293967f714b481e4bd23c7dabc25abbde


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

ceford avatar Feb 20 '24 12:02 ceford

I have tested this item :white_check_mark: successfully on e8f9963293967f714b481e4bd23c7dabc25abbde


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

Quy avatar Feb 20 '24 16:02 Quy

RTC


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

Quy avatar Feb 20 '24 16:02 Quy

I have tested this item :white_check_mark: successfully on c9f426a9d787687074e0d742a9088ce75468ee02


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

crimle avatar Feb 24 '24 13:02 crimle

I have tested this item :white_check_mark: successfully on c9f426a9d787687074e0d742a9088ce75468ee02


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

dorisdreher avatar Feb 24 '24 13:02 dorisdreher

Thank you!

bembelimen avatar Feb 28 '24 17:02 bembelimen