User tests: Successful: 0 Unsuccessful: 0
Merged in haste.
This ensures it works in RTL
Status | New | ⇒ | Pending |
Category | ⇒ | Administration Templates (admin) NPM Change |
Labels |
Added:
NPM Resource Changed
?
|
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-07-27 21:07:45 |
Closed_By | ⇒ | wilsonge |
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.
We do it the correct way like this in lots of places. Not happy you changed the pr and merged
I remember having seen both ways, so it seems we are not consistent with that.
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.
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.
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.