[#31285] - [4.0] Tidy up spacing on plugin descriptions
- Fixed in Code Base
- 8 Feb 2021
- Medium
- Build: 4.0-dev
- # 31285
- Diff
- particthistle:origin/4.0-dev-31267-pr
User tests: Successful: Unsuccessful:
Pull Request for Issue #31267
Summary of Changes
Changes<br>to<br />consistently throughout plugin language files.Changes<br>to<br><br>in various language files to add an additional line before warnings in descriptions to improve readability on screen.- Changes
<br>to paragraphs in various language files to add spacing and formatting before warnings in descriptions to improve readability on screen.
Where @gostn identified Content - Fields plugin having the issue in #31267 a closer inspection of plugins showed a number of plugins with descriptions that could be improved by additional spacing being added.
Testing Instructions
- Before applying the patch, check a few of the plugins in the list to see what the before status looks like.
- Apply the patch
- Check the various plugins listed in the file changes for this PR to see the various changes have taken effect without impacting the usability of the plugin details.
Actual result BEFORE applying this Pull Request
Before spacing is improved, there will not be a space between the first paragraph and the second paragraph:
Expected result AFTER applying this Pull Request
After spacing is improved, there will be an additional space between the first paragraph and the second paragraph:
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Language & Strings |
Sorry but the change to <br /> is not correct. They were all changed to <br> deliberately
If you believe that the spacing between the sentences should be greater (i have no opinion on this) then it must be done as paragraphs and not with double <br>
See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br for an explanation
gostn
-
Sorry but the change to
<br />is not correct. They were all changed to<br>deliberately
If you believe that the spacing between the sentences should be greater (i have no opinion on this) then it must be done as paragraphs and not with double
<br>See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br for an explanation
@brianteeman Thanks for your feedback. It's consistent with the information I have.
@particthistle Can you modify this PR to fix the issue in the right way?
Setting the "Updates Requested" label.
I have tested this item
done :)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
| Labels |
Added:
?
?
?
|
||
Apologies - I think I'd looked at a J3 language file and had seen the <br />
Reverted the changes back to use
only. Number of files edited now has dropped from 26 to 17 as several had only been the modification to <br />
If you believe that the spacing between the sentences should be greater (i have no opinion on this) then it must be done as paragraphs and not with double
<br>See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br for an explanation
This still needs to be addressed.
Question: Couldn't the spacing issue be solved with CSS?
I would go with a paragraph <p> as Brian Teeman suggested, this would show a spacing in any case, even the CSS is not loaded.
If its not fitting the styleguides, there is still the option to adjust it via CSS later.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
I would go with a paragraph <p> as Brian Teeman suggested, this would show a spacing in any case, even the CSS is not loaded.
If its not fitting the styleguides, there is still the option to adjust it via CSS later.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
@richard67
I have not considered this feature!!! I have not considered it!I have not considered this feature!!! I have not considered it!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
There are 550 instances of <br /> in the code. And 1536 instances of <br>. Why not do them all?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
Dont know where you are looking but this PR is for J4 and there should be zero instances of <br />
On the number of instances of <br /> - I searched my fork of 4.0-dev branch of the joomla-cms repo. I did not check where the instances occur: mostly in libraries/vendor, media/vendor, and node/modules. There is one occurrence in administrator/components/com_installer/tmpl/updatesite/default.php and another in configuration.php (off-line message). So I admit to making a mountain out of a molehill! Sorry.
That's not a mountain its the entire Himalaya ;)
| Labels |
Added:
?
Removed: ? |
||
| Category | Administration Language & Strings | ⇒ | Administration com_installer Language & Strings |
Let's try that again. Misinterpreted earlier comments and so had just corrected the <br> and not converted to <p>.
Now converted to paragraphs where it then improves the flow of the descriptions of the various plugins.
Also pushed a commit to correct the <br /> to <br> in the file @ceford found in:
administrator\components\com_installer\updatesites\default.php
| Labels |
Added:
?
Removed: ? |
||
| Category | Administration Language & Strings com_installer | ⇒ | Administration Language & Strings |
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
I have tested this
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
I have tested this item
I have tested this
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
@particthistle Could you apply @Quy 's suggestions (see his review comments)? Thanks in advance.
| Labels |
Added:
?
Removed: ? |
||
@richard67 @Quy - Suggestions applied.
Please removed label:Updates Requested
Ready for testing again.
| Labels |
Removed:
?
|
||
| Labels |
Added:
?
|
||
| Labels |
Added:
Conflicting Files
Removed: ? |
||
I've allowed myself to solve the conflict.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31285.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-02-08 00:40:35 |
| Closed_By | ⇒ | Quy | |
| Labels |
Added:
?
Removed: Conflicting Files |
||
Thanks!
Thanks @richard67 for fixing the final conflict and @Quy for testing again - Apologies for going dark on the last parts had my github notifications being diverted.
Sure we need the
<br>to<br />change? As far as I know, HTML5 is the default for J4, and there a stand-alone<br>is absolutely valid, and the<br />is the J3 XHTML style which we want to get rid of in J4. But I might be wrong, so hoping for more feedback.