? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
16 Oct 2021

Pull Request for Issue # .

Summary of Changes

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 '‎'.

Testing Instructions

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.

  1. On a current 3.10-dev branch or the latest 3.10 nightly build or a 3.10.2, download and install the following modified language packs and weblinks package.
  • https://test5.richard-fath.de/de-DE_joomla_lang_full_3.10.1v1.zip
    This is an unmodified but older German language pack so you will get an update to a newer version with a "normal" version number. If you have already installed a newer German language pack, uninstall it and then install the one from the link here.
  • https://test5.richard-fath.de/de-XX_joomla_lang_full_3.10.1v1_test.zip
    This is a modified German language pack which uses a different language tag so it can be installed in parallel to the German language pack mentioned before. It is modified so that both the installed and the available update versions are shown with release candidate "-rcX" version numbers.
  • https://test5.richard-fath.de/fa-XX_joomla_lang_full_3.10.2v1_test.zip
    This is a modified Persian language which uses a different language tag so it can be installed in parallel to any present Persian language pack. It is modified so that both the installed and the available update versions are shown with version numbers like e.g. "3.10.2v1". The Persian language pack for Joomla 4 currently uses such versions, for the test here with Joomla 3 it needs that modified pack.
  • https://test5.richard-fath.de/pkg-weblinks-3.9.0-rc1_test.zip
    This is a modified weblinks package to get installed and update versions shown with release candidate "-rcX" version numbers for having test for different extension types than languages.
  1. Check in the following views all version numbers which contain strings for both kinds of backend languages LTR and RTL. For RTL you can use the previously installed "Persian (Test)".
  • Extensions: Update (Use the "Clear Cache" and then the "Find Updates" button if necessary to find all updates.)
  • Extensions: Manage
  • Languages: Installed

When using Persian language you can find the views here:
j3-persian-navigate-2

  1. In database in the extensions table, delete the records for the 'de-DE' and the 'fa-XX' site languages and the weblinks editor button.
  • In phpMyAdmin using the GUI:
    j3-extensions-delete-records-for-discover-in-phpmyadmin
  • With SQL, replace #__ with your database prefix.:
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';
  1. 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.

  2. Select the discovered extensions and install them using the "Install" button.

  3. Apply the patch of this PR.

  4. Repeat steps 2 to 4.

Actual result BEFORE applying this Pull Request

Extensions: Update

LTR
j3-extensions-update_ltr

RTL
j3-extensions-update_rtl-bad

Extensions: Manage

LTR
j3-extensions-manage_ltr

RTL
j3-extensions-manage_rtl-bad

Extensions: Discover

LTR
j3-extensions-discover_ltr

RTL
j3-extensions-discover_rtl-bad

Languages: Installed

LTR
j3-languages-installed_ltr

RTL
j3-languages-installed_rtl-bad

Expected result AFTER applying this Pull Request

For LTR there should be no changes compared to before applying this PR.

For RTL the versions should be the same as for LTR.

Extensions: Update

j3-extensions-update_rtl-ok

Extensions: Manage

j3-extensions-manage_rtl-ok

Extensions: Discover

j3-extensions-discover_rtl-ok

Languages: Installed

j3-languages-installed_rtl-ok

Documentation Changes Required

None.

avatar richard67 richard67 - open - 16 Oct 2021
avatar richard67 richard67 - change - 16 Oct 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Oct 2021
Category Administration com_installer com_languages
avatar richard67 richard67 - change - 16 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 16 Oct 2021
avatar richard67 richard67 - change - 16 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 16 Oct 2021
avatar brianteeman
brianteeman - comment - 16 Oct 2021

it is a valid issue but please see #34215

avatar richard67 richard67 - change - 16 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 16 Oct 2021
avatar richard67
richard67 - comment - 16 Oct 2021

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?

avatar brianteeman
brianteeman - comment - 16 Oct 2021

Numbers should be aligned right when the language is ltr.

avatar richard67 richard67 - change - 16 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 16 Oct 2021
avatar richard67
richard67 - comment - 16 Oct 2021

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.

avatar brianteeman
brianteeman - comment - 16 Oct 2021

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;
  }
avatar richard67
richard67 - comment - 16 Oct 2021

@brianteeman In which (s)css file should the CSS be added?

avatar richard67
richard67 - comment - 16 Oct 2021

Numbers should be aligned right when the language is ltr.

@brianteeman And left when the language is rtl?

avatar brianteeman
brianteeman - comment - 16 Oct 2021

The class is complete for all directions not sure which file you want to put it in

avatar richard67
richard67 - comment - 16 Oct 2021

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.

avatar richard67
richard67 - comment - 16 Oct 2021

@brianteeman text-align: end; .. is that ok in Joomla 3.10? Or do we need to support IE11 still in 3.10?

avatar brianteeman
brianteeman - comment - 16 Oct 2021

for me its fine. it will be ignored in ie11 but still be forced ltr

avatar richard67
richard67 - comment - 16 Oct 2021

@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?

avatar brianteeman
brianteeman - comment - 16 Oct 2021

This one class will be fine for all numbers. No need to differentiate between numbers and version numbers

avatar richard67
richard67 - comment - 16 Oct 2021

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.

avatar richard67
richard67 - comment - 16 Oct 2021

@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)?

avatar richard67
richard67 - comment - 16 Oct 2021

@brianteeman Question after final question ? : What to do here? https://github.com/joomla/joomla-cms/blob/3.10-dev/administrator/components/com_installer/views/languages/tmpl/default.php#L82-L90 ... there is some title with a tooltip added before the version ... would that then be ltr, too, if we add the class to the complete <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?

avatar brianteeman
brianteeman - comment - 16 Oct 2021

in this case just a span

avatar richard67
richard67 - comment - 16 Oct 2021

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?

avatar infograf768
infograf768 - comment - 17 Oct 2021

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.

avatar richard67
richard67 - comment - 17 Oct 2021

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

https://github.com/joomla/joomla-cms/blob/3.10-dev/administrator/components/com_installer/views/languages/tmpl/default.php#L82-L90

and here

https://github.com/joomla/joomla-cms/blob/3.10-dev/administrator/components/com_languages/views/installed/tmpl/default.php#L102-L110

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?

avatar richard67
richard67 - comment - 17 Oct 2021

@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.

avatar brianteeman
brianteeman - comment - 17 Oct 2021

can you update the pr with your new changes and then I can take a look after lunch

avatar richard67
richard67 - comment - 17 Oct 2021

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.

avatar richard67 richard67 - change - 17 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 17 Oct 2021
avatar richard67
richard67 - comment - 17 Oct 2021

@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.

avatar brianteeman
brianteeman - comment - 17 Oct 2021

Just make the class more specific

.table td.number {
	direction: ltr;
	text-align: end;
}

avatar richard67
richard67 - comment - 17 Oct 2021

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.

avatar richard67 richard67 - change - 17 Oct 2021
Title
[3.10] [WiP] Use LTR marks for extension versions to fix RTL display of versions which contain characters
[3.10] Use LTR marks for extension versions to fix RTL display of versions which contain characters
avatar richard67 richard67 - edited - 17 Oct 2021
avatar richard67
richard67 - comment - 17 Oct 2021

The PR works as it is and testing instructions are complete, so I mark it as ready and then the release lead may decide.

avatar richard67
richard67 - comment - 17 Oct 2021

P.S.: We have the same issue also in J4 and this PR could be merged up later as it is, so maybe not only @zero-24 but also @wilsonge should decide and let me know if I shall close it or not.

avatar brianteeman
brianteeman - comment - 17 Oct 2021

this is a much bigger issue than shown here and it happens everywhere there is a "number with latin characters"

image

avatar richard67
richard67 - comment - 17 Oct 2021

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?

avatar brianteeman
brianteeman - comment - 17 Oct 2021

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

avatar richard67
richard67 - comment - 17 Oct 2021

@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 .

avatar brianteeman
brianteeman - comment - 17 Oct 2021

technically we dont as the version could just be "alpha" or "ألفا" - it doesn't have to have a number in it (does it?)

avatar richard67
richard67 - comment - 17 Oct 2021

@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.

avatar richard67
richard67 - comment - 17 Oct 2021

Or we use the <bdi> with a dir=ltr attribute.

avatar richard67
richard67 - comment - 17 Oct 2021

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.

avatar richard67
richard67 - comment - 17 Oct 2021

@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.

avatar richard67 richard67 - change - 17 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 17 Oct 2021
avatar richard67
richard67 - comment - 17 Oct 2021

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.

avatar richard67 richard67 - change - 17 Oct 2021
The description was changed
avatar richard67 richard67 - edited - 17 Oct 2021
avatar richard67
richard67 - comment - 17 Oct 2021

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.

avatar richard67 richard67 - close - 17 Oct 2021
avatar richard67 richard67 - change - 17 Oct 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-10-17 19:12:57
Closed_By richard67
Labels Added: ?
avatar richard67
richard67 - comment - 17 Oct 2021

I'll keep the branch of this PR for a while in case if for some reason we have to come back to it.

Add a Comment

Login with GitHub to post a comment