Machine name generation is way too slow

Could the form submission be disabled until the machine name has been generated? I wouldn't say it's 'way too slow'. It's slower than someone typing fast and pressing Return quickly.
Having to wait 1-2 seconds for it to finish its work so you can submit the form is ok UX I think. It's a huge improvement on submitting the form and having a truncated machine name which then means you have to delete and start again.

We are looking into potentially using the machine name field in 📌 List key|label entry field is textarea, which doesn't give guidance towards the expected input Fixed which would mean that you would potentially fill out multiple machine name fields on a single page. In this case, even a few second delay could be quite jarring.

The PR from @Utkarsh_33 is trying to re-implement the machine name generation on the client side, which would mean that the machine name could be generated almost immediately, and it wouldn't depend on latency.

So tried implementing the machine name generation at the client side using the Transliterationlibrary but the major problem while using this library is that it does not take the different languages into consideration while translating the same text.
For example the test named providerTestMachineNameController() fails when we change the langcode.

It looks like the library does have an option for language overrides. The replace property in the option allows overriding replacement patterns for specific characters: https://github.com/dzcpy/transliteration#transliteratestr-options. It is not per language, but we should be able to implement the language aware overrides on our side. This seems worth investigating at least.

Seems MR has some failures.

Discussed with @catch on Slack. It seems on principle the approach being proposed here is fine, since there's no way we could make the UX similar using the ajax based architecture. @catch hasn't had a chance to take a look at the library, but he didn't raise any concerns so far. The issue seemed low risk since we could always fallback to the ajax based approach if something goes wrong.

He had two questions which we could answer to by providing some additional test coverage:

Whether/how this works with non-latin characters like CJK, arabic, persian vs. the PHP implementation. Would be good to add one or two to the test coverage.

And also whether there's any chance the library creates an invalid machine name that would fail the PHP validation (seems like not though).

Yes this seems like a good approach to me - it's just a helper that you can override, and there's no way we can make the speed of AJAX match not having AJAX at all. We also don't do any AJAX validation here afaik, just transliteration, so not losing anything there (checking for duplicates etc.).

A couple of small things, and it looks like there still needs to be test cases added to core/tests/Drupal/FunctionalJavascriptTests/MachineName/MachineNameTransliterationTest.php from \Drupal\Tests\Component\Transliteration\PhpTransliterationTest::testPhpTransliteration (one of the unresolved threads)

For the CC failure

Test MR but going to admin/people/roles/add typing something real fast and pressing enter. New package also appears to have gone through, at least first round, of approval.

Looks like unresolved threads in the MR.

Ah you are correct. My mistake.

Appears all threads have been answered.

Looks like CI is failing on a relevant test case

All green with that change. Back to RTBC

Overall this looks encouraging.

Is the AJAX transliteration controller unused now, and could we deprecate both the controller and the route for removal in 11.x?

Back to needs review. If we don't need the route/controller any more my preference would be to deprecate them in this issue.

Laurii said someone is working on this just moving it so it doesn't get re-reviewed until the changes happen.

tim.plunkett → made their first commit to this issue’s fork.

This should pass now. Had to move the deprecation to the method

All green. Ran same test I did before and still appears to work. Moving back to RTBC.

I updated the deprecation message to deprecate this in 10.2.0 since I think it's too late to backport this to 10.1.x. I also removed the mention of Drupal.behaviors.machineName.transliterate to avoid recommending something that isn't necessarily intended as an API. We may want to ask the framework managers if we want to move Drupal.behaviors.machineName.transliterate into it's own API in future.

The previous commits revert few changes that were out of scope and not strictly required for the fix. I opened 📌 Generate machine names even faster Fixed as a follow-up.

Confirmed with @catch on Slack that he's fine with the library addition since that wasn't explicitly confirmed on the issue.

Committed 570710a and pushed to 11.x. Thanks!

AWESOME work here!

You know what would be a great complementary commit?

📌 Add config validation for the allowed characters of machine names Fixed

Then we'd have solved all fundamental problems with machine_names in 10.2.x! 😊🤞

Also note ✨ Strip useless "_" at beginning and end of JS-transliterated machine names Fixed which was affected by these changes.

@laurii:
This was committed to 11.x and marked fixed, but the deprecation message says:

* @deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. There is no

Doesn't this need to be backported / committed to 10.2.x then, or am I missing something? Or is the comment wrong?
Just wanted to ensure it's not forgotten... :)

@Anybody see https://www.drupal.org/about/core/blog/new-drupal-core-branching-scheme-... → - 10.2.x will eventually be branched from 11.x, the branch is not open yet.

Automatically closed - issue fixed for 2 weeks with no activity.

lauriii → closed merge request !3786