User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Work in Progress: I might replace this PR soon by one doing the same thing in another way. For now I put this one here in draft status.
Preceed version numbers of extensions with LTR marks '‎'
in diverse backend views for correct display of versions which contain characters (e.g. development, alpha, beta or RC versions, or versions like "1.2.3.v4" like they are used by some Joomla 4 language pack) with RTL languages, like we do it already in the "Live Update" tab of the Joomla Update component for the installed Joomla version and the latest Joomla version.
Currently such versions are shown wrong when current language in backend has writing direction RTL.
In RTL language version numbers or dates in western formats are still shown with LTR direction, that's the use of the LTR marks '‎'
.
The change in file administrator/components/com_installer/views/languages/tmpl/default.php can be only tested by code review since I haven't found a hack to make the installer find the modified language packs used during the other tests below.
When using Persian language you can find the views here:
DELETE FROM `#__extensions` WHERE `type` = 'language' AND `element` = 'de-DE' AND `client_id` = 0;
DELETE FROM `#__extensions` WHERE `type` = 'language' AND `element` = 'fa-XX' AND `client_id` = 0;
DELETE FROM `#__extensions` WHERE `type` = 'plugin' AND `element` = 'weblink' AND `folder` = 'editors-xtd';
Go to the "Extensions: Discover" view and check the version numbers which contain strings. Use the "Discover" button if necessary to discover all extensions. Check with LTR and RTL backend languages.
Select the discovered extensions and install them using the "Install" button.
Apply the patch of this PR.
Repeat steps 2 to 4.
For LTR there should be no changes compared to before applying this PR.
For RTL the versions should be the same as for LTR.
None.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_installer com_languages |
it is a valid issue but please see #34215
@brianteeman What do you mean with that? Shall I also prefix the column heading with the LTR marks?
Update: I don't think so, and I don't see how that issue is related to my PR here. It's a completely different thing, or am I missing something?
Numbers should be aligned right when the language is ltr.
Numbers should be aligned right when the language is ltr.
@brianteeman That's right, but this PR doesn't change anything on the alignment, see the screenshots. So I would say the alignment should be done with another PR.
Just add a new class and fix it all in one go instead of adding this cryptic code that few people understand everytime
.number {
direction: ltr;
text-align: end;
}
@brianteeman In which (s)css file should the CSS be added?
Numbers should be aligned right when the language is ltr.
@brianteeman And left when the language is rtl?
The class is complete for all directions not sure which file you want to put it in
The class is complete for all directions not sure which file you want to put it in
@brianteeman Not sure either. I think it should be something global and not template-specific but I have no idea what would be the best place for it.
@brianteeman text-align: end;
.. is that ok in Joomla 3.10? Or do we need to support IE11 still in 3.10?
for me its fine. it will be ignored in ie11 but still be forced ltr
@brianteeman So I think I will later or tomorrow replace this PR by another one where I do the same thing as here for version "numbers" but using that CSS, and for other numbers like ID, package ID, ... one (you or me) make one or more separate PRs. I think we should not do all together in one PR because that would complicate testing instructions. But maybe your have a different opinion?
This one class will be fine for all numbers. No need to differentiate between numbers and version numbers
This one class will be fine for all numbers. No need to differentiate between numbers and version numbers
@brianteeman Yes, I understood that, but there is no need to do all the places in one PR. Of course if you want to do that, feel free to do so, but I don't want to make such a bit PR with such long testing instructions. For the version numbers I have my instructions and screenshots ready.
Update: Ok, the screenshots I couln't re-use because of the alignment change ... but I still think we could make several PRs, first one which introduces the CSS and others which fix things in small packages which are easy to test.
@brianteeman Final question: The class could be applied on the <td>
element? If so: Should the corresponding <th>
also have text-align: end
(but not direction: ltr
)?
@brianteeman Question after final question <td>
? Maybe it needs here to wrap only the version into a span in that case and apply the "Number" class only to that span?
in this case just a span
in this case just a span
@brianteeman Could you propose code for that part? If the title with the tooltip would inherit the class of the span, would it then need another span inside that span?
BT proposal would also work.
But only when the number containing a latin letter is NOT included in a phrase with RTL glyphs.
For example, it will never work for <span class="visually-hidden"><?php echo Text::sprintf('MOD_VERSION_CURRENT_VERSION_TEXT', $version); ?></span>
in mod_version.
For example, it will never work for
<span class="visually-hidden"><?php echo Text::sprintf('MOD_VERSION_CURRENT_VERSION_TEXT', $version); ?></span>
in mod_version.
@infograf768 I think in mod_version we won't have strange version numbers like "1.2.3-rc1" or "1.2.3.v4", and if they are pure numeric, like "1.2.3.4" they will be ltr in any case, without CSS or LTR mark, as far as I could see.
For me it is more a question how to do it e.g. here
and here
If we add the "number" (or whatever) CSS class to the <td>
or to the span
which has the title for the tooltip, won't the tooltip then inherit the LTR direction, too? If so, what's the way out? An empty span for the tooltip and then a span with the "number" after the first span? Or inside the first span?
@brianteeman Do you have a suggestion for that?
@brianteeman I can get the direction working, i.e. the issue handled here, but not the alignment. It seems that is superseded somehow with CSS from template-rtl.css. I have recompiled both, template.css and template-rtl.css. When using an LTR language, the version numbers e.g. in the update view are still left aligned, and with an RTL language right aligned.
can you update the pr with your new changes and then I can take a look after lunch
can you update the pr with your new changes and then I can take a look after lunch
@brianteeman I'd prefer not to change this PR as long as I don't know if the other way is the way to go.
You can easily test it by adding the "number" class e.g. to the td in the update view and add your css where you said and complie the 2 less files.
@brianteeman But I still don't see why we should not use this PR as it is. We use the LTR marks already at other places. In the browser you will never see them when inspecting. Someone who edits PHP code here will find out what it means after a quick search in Google. Alternatively, if we have dedicated elements like e.g. spans which contain only the version number, we could add the dir attribute to that span, that would be more readable maybe.
If you really want to solve the issue here and the alignments for numbers with CSS in one PR, I suggest you do that PR because it might be beyond my skills, then I can close this one here in favour of yours if yours works.
Just make the class more specific
.table td.number {
direction: ltr;
text-align: end;
}
I still think this PR as is it fixes the issue with direction independently from any template, and the alignment is something to be fixed elsewhere.
Title |
|
The PR works as it is and testing instructions are complete, so I mark it as ready and then the release lead may decide.
this is a much bigger issue than shown here and it happens everywhere there is a "number with latin characters"
@brianteeman Does that change anything on the validity of this PR?
I still dont like using the cryptic string and neither does the w3c https://www.w3.org/International/questions/qa-bidi-controls
That should be used only when you can't control the markup - which we can do either with a css class as I described before OR using the bdi element
<bdi> (3.10.0-rc2-dev)</bdi>
Further reading
https://www.w3.org/International/articles/inline-bidi-markup/#whattodo
@brianteeman Should it not better be a <span dir="ltr">
? The <bdi>
is described here for being used if we don't know the direction: https://www.w3.org/International/articles/inline-bidi-markup/#html4markup .
technically we dont as the version could just be "alpha" or "ألفا" - it doesn't have to have a number in it (does it?)
@brianteeman But as far as I can read here the <bdi>
element means that it shall have the opposite direction than the surrounding stuff: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/bdi
And for the versions we handle here it should be always LTR.
Theoretically we could have versions like "alpha" or "ألفا", too, and in this case nobody can know how that should be shown.
Practically we will have versions like handled here in this PR with the modified packages for testing and like it can be seen in Joomla 4 without modified package for the Persian language pack.
So I think if we want to avoid the unicode thing because we can and know the direction, we should use dir="ltr"
in the markup.
Or we use the <bdi>
with a dir=ltr
attribute.
Or maybe I don't fully understand it yet and it will work like it should. I will see if I can test that later.
@brianteeman <bdi>
(without any dir
attribute) seems to work for the issue handled by this PR here. So if that's the way to go I can make a replacement PR to this one here.
I've just noticed that my description was wrong regarding where we already have the LTR marks (and should replace them when making a PR for using bdi instead). I had written that it was the extension details in the pre-update checker, but that was wrong. It is the versions shown in the Live Update tab of the Joomla update component.
In the pre-update checker we have the issue handled here, too, and it might have to be fixed in js for that place.
Closing in favour of PR #35847 . @brianteeman Please check even if it's still draft. I have have to add the fix at a few more places in that PR or others, but you could already check what is there so I can see it goes the right way.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-10-17 19:12:57 |
Closed_By | ⇒ | richard67 | |
Labels |
Added:
?
|
I'll keep the branch of this PR for a while in case if for some reason we have to come back to it.
it is a valid issue but please see #34215