NPM Resource Changed ? Pending

User tests: Successful: 0 Unsuccessful: 0

avatar brianteeman
brianteeman
27 Jul 2021

Merged in haste.

This ensures it works in RTL

avatar brianteeman brianteeman - open - 27 Jul 2021
avatar brianteeman brianteeman - change - 27 Jul 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jul 2021
Category Administration Templates (admin) NPM Change
avatar richard67 richard67 - test_item - 27 Jul 2021 - Tested successfully
avatar richard67
richard67 - comment - 27 Jul 2021

I have tested this item successfully on 25d77a0

Code review only, but that should be sufficient here.


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

avatar wilsonge wilsonge - change - 27 Jul 2021
Labels Added: NPM Resource Changed ?
avatar wilsonge wilsonge - change - 27 Jul 2021
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-07-27 21:07:45
Closed_By wilsonge
avatar wilsonge wilsonge - close - 27 Jul 2021
avatar wilsonge wilsonge - merge - 27 Jul 2021
avatar wilsonge
wilsonge - comment - 27 Jul 2021

Removed the dir=ltr because we don't have that in other places and technically if dir is absent it gives a fallback (not sure practically on our template if that can ever happen but better safe than sorry). It's probably technically the right thing - but if we do it here we should probably do it all over the shop rather than in this one place.

avatar brianteeman
brianteeman - comment - 27 Jul 2021

We do it the correct way like this in lots of places. Not happy you changed the pr and merged

avatar richard67
richard67 - comment - 27 Jul 2021

I remember having seen both ways, so it seems we are not consistent with that.

avatar brianteeman
brianteeman - comment - 27 Jul 2021

yes - everything created by one person was done the wrong way or just shoved in a separate rtl file. Which is why there were so many bugs to fix as you don't know there is a need to update in other places. bad practice leads to bad code leads to more work.

avatar wilsonge
wilsonge - comment - 27 Jul 2021

I just searched and we currently use dir=ltr in 3 places in all the SCSS twice in joomla-tab scss override and once in quickicons. I very specifically said that what you did is probably technically right - just it's not good to just add it to one more place - it should be done at once or not at all.

Add a Comment

Login with GitHub to post a comment