Skip to content

Conversation

@AndySDH
Copy link
Contributor

@AndySDH AndySDH commented Jun 9, 2019

Summary of Changes

This PR improves checks for empty output when rendering a field as part of field groups. Some checks were already there but not thorough, and empty output could occur in rare cases.

This makes sure that a field that has an empty value or an empty output does not get displayed in all cases.

One check comes from the original subfields PR (https://github.com/joomla/joomla-cms/pull/22446/files#diff-02a314737553eb71a8516e925b220cce) that was postponed to Joomla 4.
Another extra check I added myself.

Testing Instructions

Not much needed really. Just double checks.

Expected result

Field with empty output should never display

Actual result

Field with empty output could be displayed in rare occurrences

Documentation Changes Required

None

@AndySDH
Copy link
Contributor Author

AndySDH commented Jun 9, 2019

Updated with @Quy suggestions, thanks!

@Quy
Copy link
Contributor

Quy commented Jun 9, 2019

I have tested this item ✅ successfully on ff61169


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

1 similar comment
@richard67
Copy link
Member

I have tested this item ✅ successfully on ff61169


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

@Quy
Copy link
Contributor

Quy commented Jun 9, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 9, 2019
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on d6c16be


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

@AndySDH
Copy link
Contributor Author

AndySDH commented Jun 12, 2019

To merge in 3.9.8?

Edit: 3.9.9 now I guess :D

@Quy
Copy link
Contributor

Quy commented Jun 12, 2019

@AndySDH Please apply the suggested changes. It will not require retesting.

@AndySDH
Copy link
Contributor Author

AndySDH commented Jun 12, 2019

I wasn't too sure the suggested changes made much sense as there's no need to compare for data type when we are comparing to an empty string.

But ok, I applied them.

@AndySDH
Copy link
Contributor Author

AndySDH commented Jun 25, 2019

Reminder about this for 3.9.9

@ghost
Copy link

ghost commented Jun 25, 2019

Reminder about this for 3.9.9

calling Release Lead @HLeithner to be sure.

@HLeithner HLeithner merged commit e7c13df into joomla:staging Jun 25, 2019
@HLeithner
Copy link
Member

thx

@HLeithner HLeithner added this to the Joomla 3.9.9 milestone Jun 25, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 25, 2019
@ghost
Copy link

ghost commented Jul 5, 2019

@AndySDH can you please mail me [email protected]

@AndySDH
Copy link
Contributor Author

AndySDH commented Jul 5, 2019

@AndySDH can you please mail me [email protected]

I did

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants