Skip to content

Comments

[4.0] Implementing display of fieldset descriptions for fieldset children#30450

Merged
rdeutz merged 4 commits intojoomla:4.0-devfrom
infograf768:4.0_child_fieldset_description
Aug 27, 2020
Merged

[4.0] Implementing display of fieldset descriptions for fieldset children#30450
rdeutz merged 4 commits intojoomla:4.0-devfrom
infograf768:4.0_child_fieldset_description

Conversation

@infograf768
Copy link
Member

@infograf768 infograf768 commented Aug 22, 2020

Pull Request for Issues #30433 & #30431

Summary of Changes

Added the possibility to display fieldsets description for children (and parent when there are children)

Testing Instructions

Structure of a xml with children fieldsets

<fieldset name="filtering" label="MOD_ARTICLES_CATEGORY_FIELD_GROUP_FILTERING_LABEL" description="Top most description">

<fieldset name="filtering_child1" label="Child 1" description="Child description 1">
[some fields]
</fieldset>
<fieldset name="filtering_child2" label="Child 2" description="Child description 2">
[the remaining fields]
</fieldset>

</fieldset>

For example, replace the file modules/mod_articles_category/mod_articles_category.xml where I have made such a structure.
with this one (unzip first)
mod_articles_category.xml.zip

Test, then patch and run NPM as we have a small scss change.

Actual result BEFORE applying this Pull Request

Screen Shot 2020-08-22 at 09 38 41

Expected result AFTER applying this Pull Request

Screen Shot 2020-08-22 at 09 36 52

@obuisard
Please test and add your test result on https://issues.joomla.org/tracker/joomla-cms/30450

@obuisard
Copy link
Contributor

I have tested this item ✅ successfully on 151b0ac

Perfect, thank you!


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

echo '<legend>' . $label . '</legend>';

// Include the description when available
if (isset($fieldSet->description) && trim($fieldSet->description))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (isset($fieldSet->description) && trim($fieldSet->description))
if (!empty($fieldSet->description))

@infograf768
Copy link
Member Author

infograf768 commented Aug 23, 2020

@Bakual
See #3048

EDIT: in fact that code is much much older...

Any reason to use
if (isset($fieldSet->description) && trim($fieldSet->description))
instead of @Quy proposal above
if (!empty($fieldSet->description))

@Bakual
Copy link
Contributor

Bakual commented Aug 23, 2020

I haven't changed the code itself in that old PR, I just moved it to a different place. So I wasn't thinking about why the trim is there.

There would be a small difference between the code, that is when the description holds only a whitespace. The first line wouldn't show it (because trim removes the space), the second line would show it.

Personally, I would go with !empty, I think we can live with the difference in behavior.

@infograf768
Copy link
Member Author

@Quy @obuisard @richard67
Modifs done. Can be tested again.

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

I have tested this item ✅ successfully on 83c9d5b


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

1 similar comment
@obuisard
Copy link
Contributor

I have tested this item ✅ successfully on 83c9d5b


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

@infograf768
Copy link
Member Author

Back to RTC

Appveyor failure is unrelated.


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 23, 2020
@infograf768 infograf768 added this to the Joomla 4.0 milestone Aug 23, 2020
@infograf768
Copy link
Member Author

Took off an extraneous new line at the end of _alerts.scss to force rebuild to solve appveyor failure to run.
No change for tests and RTC

@rdeutz rdeutz merged commit 46e2486 into joomla:4.0-dev Aug 27, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 27, 2020
@infograf768 infograf768 deleted the 4.0_child_fieldset_description branch August 27, 2020 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants