- @ifrik opened merge request.
- @ifrik opened merge request.
- Status changed to Needs review
almost 2 years ago 4:27pm 28 January 2023 - 🇮🇳India pooja saraah Chennai
Fixed failed commands on #31
Attached patch against Drupal 10.1.x - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
@pooja saraah There is no interdiff: what changed? And: could you please just commit those changes to the merge request? 😊🙏
- 🇳🇱Netherlands ifrik
There are some coding standard errors in the branch that I pushed yesterday and I'll fix those myself.
- Status changed to Needs work
almost 2 years ago 5:25pm 30 January 2023 - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Did a detailed review — found a few small problems. Most importantly: the absence of RTL test coverage!
- Status changed to Needs review
almost 2 years ago 1:57pm 3 February 2023 - 🇳🇱Netherlands ifrik
Obviously it fails if I only change half of the test..
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Still one last test failure 😅
And … AFAICT the feedback has not yet been addressed?
- First commit to issue fork.
- Status changed to Needs work
almost 2 years ago 8:20pm 12 February 2023 - Assigned to ifrik
- 🇳🇱Netherlands ifrik
Hopefully that fixed the issue of the text direction.
The changes for the functional test are still underway. - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Marked 🐛 CKEditor5 language selector unusable Closed: duplicate and ✨ Add option to use website languages as the CKEditor5 drop-down languages Closed: duplicate as duplicates.
Given the wide interest in this (which is somewhat surprising given that the current functionality exactly matches that of CKEditor 4 in Drupal 8 & 9!), bumping to .
- 🇺🇸United States charles belov San Francisco, CA, US
Wim Leers → credited Charles Belov → .
- 🇺🇸United States charles belov San Francisco, CA, US
Updating title from CKEditor 4 to CKEditor 5.
- 🇺🇸United States charles belov San Francisco, CA, US
Apologies for not diving in and coding. I don't have the bandwidth to set up a test environment and a way of applying my code.
For rigorousness, would it be better if the test language were not one of the UN six?
Currently the test is add English and French, then test that choosing enabled produces English and French.
I respectfully suggest the following:
Add English and a language that is in the all list but not in the UN list, for example, Chinese (Traditional), as website languages.
Choose Enabled for CKEditor languages.Then test:
- that English is in the CKEditor list (testing a UN language which is a website language)
- that Chinese (Traditional) is in the CKEditor list (testing a non-UN language which is a website language)
- that Chinese (Simplified) is not in the CKEditor list (testing a UN language which is not a website language)
- that Portuguese is not in the CKEditor list (testing a non-UN language which is not a website language) - 🇺🇸United States charles belov San Francisco, CA, US
Note that I made major edits to the previous comment. If you receive comments via email, please check the current version.
- 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
In 🐛 Add option to select from Drupal's enabled languages to CKEditor "Language" button Closed: duplicate I added to the list of duplicates of this ticket, then discovered this one.
Looks like the MR here wants a rebase?
One thing on my mind is the language in the selector - "All 107 languages" doesn't include any custom languages configured on the site, so I feel a more accurate term is "Drupal predefined languages". "All" is a misdirecting term when it may not include any custom languages of the site itself, and there are many languages not present in LanguageManager::getStandardLanguageList().
In the WIP-until-just-now MR at https://git.drupalcode.org/project/drupal/-/merge_requests/4655 I had something like this for the options:
- United Nations official languages
- Predefined languages (107 languages)
- Enabled languages (2 languages
and the description text:
"Languages to show in the language dropdown. United Nations official languages are the six official languages of the UN. Predefined languages are the @count_predefined predefined languages in Drupal. Enabled languages are the @count_enabled languages currently configured in Drupal, including site custom languages."
- last update
over 1 year ago 30,061 pass - @xurizaemon opened merge request.
- 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
I didn't want to mess up any work on the existing MR, so I've opened a fresh MR against 11.x with a quick rebase. Sharing mostly to save someone else having to repeat dealing with the conflicts that popped up for me: !4656.
I hope to take a closer look at this during the week.
- last update
over 1 year ago 30,061 pass - Issue was unassigned.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Thanks for pushing this forward again, @xurizaemon! Looking forward to reviewing this when you think it's ready for that 😊
- Assigned to xurizaemon
- 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
I will have a shot at that @ifrik!
One thing I'd like to propose is changing the language. I feel strongly that "all" is not an appropriate term here (as noted in #59 above), so in my MR I intend to provide the following changed list of options:
Input welcome! My intent is to remove unclear language in using "all", and to standardise the output.
I plan to address this language change, only showing the third option when appropriate (site has languages configured), and test coverage.
- last update
over 1 year ago 30,135 pass - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 30,124 pass, 2 fail - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
Changing title and ID to reflect proposed "configured" value, not "enabled". This more accurately reflects the language used at
/admin/config/regional/language
Have added some tidy up and refactor to my MR - trying to be a good scout here, happy of course to back out any changes which go beyond what is appropriate. The existing tests had one very long method which did a lot, and it was hard to follow the location of the browser moving through the test. Hoping the changes will make this clearer.
- last update
over 1 year ago 30,136 pass - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
Earlier I'd said "Only show the third option when the site has configured languages"
But ... all sites have a minimum of one language, so this doesn't happen. What can happen is that the site doesn't have Language module enabled, but the site still shows "Site configured languages (1 language)", and I feel that hiding the option in this case would be less discoverable, so I left it as-is.
If Language module is enabled, then the settings screen description now links to the route which lets admins configure languages.
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 30,127 pass, 2 fail - Status changed to Needs review
over 1 year ago 9:25pm 3 September 2023 - last update
over 1 year ago 30,127 pass, 2 fail - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
I already thought that "all" was a poor label for the existing configuration value which refers to Drupal's standard languages. Perhaps "standard" might be better there, but I think that change is out of scope for this issue.
Considering future approaches such as the one mentioned in 🐛 CKEditor5 language selector unusable Closed: duplicate I think "configured" would be a similarly poor label for the one we're introducing here. I think this label should be much clearer, eg "site_configured". ($languageManagerInterface->getLanguages() calls this "a list of languages set up on the site", so we have "enabled", "set up", "configured" but the important detail seems to be that this is the site configuration, where the idea in #3375157 might be an input format configuration.
I'm not touching #3375157 here except to avoid using a label which would make introducing that less clear. Nor am I touching on "all" as it's pre-existing. I would appreciate input on improvements on "site_configured" (because I might be missing some nuance around that label).
- last update
over 1 year ago 30,136 pass - last update
over 1 year ago 30,137 pass - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
Screenshots -
Options as displayed on text format settings CKEditor > Language tab:
Description as shown on text format CKEditor Language tab:
Language selector with site's configured languages as options:
- Status changed to RTBC
over 1 year ago 7:48pm 8 September 2023 - 🇺🇸United States smustgrave
Have an Umami setup currently running, which has 2 languages installed
Applied MR 4656
Verified I see the 3rd option in the language settings for the text format.
Created a node and verified the language dropdown shows just 2 languages.LGTM!
- last update
over 1 year ago 30,147 pass - last update
over 1 year ago 30,147 pass - Issue was unassigned.
- last update
over 1 year ago 30,151 pass - last update
over 1 year ago 30,155 pass - last update
over 1 year ago 30,162 pass - Status changed to Needs work
over 1 year ago 11:34am 18 September 2023 - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Looks awesome!
I'm not a native speaker, but shouldn't it be ?
anyway for complying with one upcoming coding standard change, which if it'd land first would cause this MR to fail tests.
- First commit to issue fork.
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 30,165 pass - Status changed to Needs review
over 1 year ago 5:33pm 18 September 2023 - Status changed to RTBC
over 1 year ago 10:09am 19 September 2023 - last update
over 1 year ago 30,169 pass - last update
over 1 year ago 30,169 pass - 🇺🇸United States charles belov San Francisco, CA, US
I'm having trouble testing this due to 🐛 Eliminate or improve error upon adding the language plug-in Active and 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info . Is there some other prerequisite for this? I don't want to derail this patch given that the two issues listed occur for me with or without the patch but is there something I'm missing?
- 🇺🇸United States charles belov San Francisco, CA, US
I've related 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info because the issue documented there occurs if I test the current issue. However, it is not a child of this issue because the issue documented there also occurs if I choose 107 languages and UN languages. However, I have a workaround, documented in that issue, which works if I default to the UN languages but not if I choose 107 languages. But the workaround also doesn't work if I install the patch for the current issue and choose site-configured languages.
Steps for that issue when testing with the patch in the current issue:
1. Go to https://simplytest.me/
2. In the field Evaluate Drupal Projects, type Drupal
3. Choose Drupal core
4. Accept 10.1.3 (or whatever the latest version offered is) as the default
5. Click Advanced options
6. In the field Add patches on the chosen project, paste https://git.drupalcode.org/project/drupal/-/merge_requests/4656.patch
7. Click Launch sandbox
8. Wait to be redirected to the test instance
9. Enter the admin user name and password
10. Click Extend
11. Under Multilingual, check Content Translation and Language
12. Click Install
13. Wait until the process is done
14. Click Configuration, then Languages
15. Click Add language
16. In the Language name drop-down, choose Custom Language
17. Type tl (that is, lower-case L) for the Language Code and Filipino for the Language Name.
18. Click Add Custom Language
19. Click Configuration, then Text formats and editors
20. On the row for basic HTML, click Configure
21. Drag the Language button into the active toolbarResult: Warning message
The Language plugin needs another plugin to create , for it to be able to create the following attributes: . Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it.22. Under CKEditor5 plugin settings, click Language
23. In the drop-down, choose Site-configured languages (2)
24. Click Save ConfigurationResult: Error message
The Language plugin needs another plugin to create , for it to be able to create the following attributes: . Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it.Attempted workaround:
25. Under CKEditor5 plugin settings, click Source editing
26. Change<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id>
to read
<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <span lang dir>
27. Click Save Configuration
Expected results:
- Error message disappears.
- Source editing setting continues to read:
<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <span lang dir>
Actual results:
- Error message remains.
- Source editing setting reads:
<cite> <dl> <dt> <dd> <a hreflang> <blockquote cite> <ul type> <ol start type> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id>
- 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
#81: Now that you mention it, I recall hitting that behaviour as well! However I believe I ran into it before applying the changes in this issue, when initially setting up the Language of Parts plugin. So 🐛 Following instructions for error upon adding the language plug-in does not remedy the issue Postponed: needs info for me isn't a blocker or issue here.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
#81: that's unrelated to this issue. Just responded at #3388978-5: Eliminate or improve error upon adding the language plug-in → — let's address that usability problem there, let's not derail this issue 🙏 (with your admittedly wonderfully detailed steps to reproduce! 👍)
- 🇺🇸United States charles belov San Francisco, CA, US
Wim Leers kindly pointed my misunderstanding of how adding plugins is supposed to work 🐛 Eliminate or improve error upon adding the language plug-in Active on my issue 🐛 Eliminate or improve error upon adding the language plug-in Active . I corrected step 26 in my preceding comment on the current issue ✨ Third option for the CKEditor 5 "Language" button: `site_configured` (in addition to `un` and `all`) RTBC from
<span lang dir>
to<span>
and the steps worked!Adding one more vote to reviewed and tested by the community.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
🥳 Now let's get that UI string improved in 🐛 Eliminate or improve error upon adding the language plug-in Active ! 🤝
- last update
over 1 year ago 30,206 pass - last update
over 1 year ago 30,209 pass - last update
about 1 year ago 30,336 pass, 2 fail - Status changed to Needs review
about 1 year ago 7:44am 28 September 2023 - Status changed to RTBC
about 1 year ago 8:54am 28 September 2023 - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
Moving back to RTBC per my response on MR. @laurii, I don't think that should prevent this useful fix landing.
- Status changed to Fixed
about 1 year ago 9:03am 28 September 2023 - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
Thanks! Happy to see this in :)
Automatically closed - issue fixed for 2 weeks with no activity.
Hi,
I see that this is fixed but it's been released on the 11.x-dev branch. Could someone make available a patch for Drupal 10 in the meantime?
Thank you
- Status changed to Fixed
8 months ago 5:46am 16 April 2024 - 🇳🇿New Zealand xurizaemon Ōtepoti, Aotearoa 🏝
@tariqDev I suggest to try the patch available from https://git.drupalcode.org/project/drupal/-/commit/e1fc76a1d2d9ff925bdbb... which may apply fine to your 10.x site.