Problem/Motivation

This patch removes *all* the custom one off code for config translation of date formats because locale module already handles that perfectly for *shipped* date formats. Config translation has auto generated UIs that cover both shipped and custom created config. We can improve DX by removing the customized solution here and reusing the system provided one.

TL;DR: Drupal 8 has two competing built-in user interfaces to localize date formats. None of them actually allow you to translate/localize formats. Then there is a third one that allows localization but is very hard to use and only works for shipped formats. The combination of them works even worse.

Let's see what they can do:

Can localize both on the edit tab and on the localize tab! "Localize" is named in a non-standard way also, nowhere else do we use this label for this task.

If you go into Edit, you can actually associate multiple languages with a format. That sounds cool in theory, but if you want different value for a language, you cannot enter that. Also you cannot save the same machine name with a different value for a different language, so no way to *actually* localize your date format. You can just declare which languages a format applies to (which then is not really used to eg. exclude formats from a list or things like that AFAIK). The different languages are also saved as translations, which directly works with locale override files crossing over to the concerns of that module:

So you want to actually localize your date formats! Go localize tab! Lots of promise here! :)

Cool, let's localize to Hungarian then! As you can see this screen is not that bad, it displays even hidden date formats, and lets you assign different values to them per language. At least that is the promise. Problems incldue no freeform picking, so eg. no way to actually localize in a way that would fit the Hungarian date formats. Also, no way to translate the label of the date format name which could show up on eg. a content submission UI. Also, this form actually does not work, it does not save the localized value. Why? Well, the key is in system_date_format_localize_form_save() and the submission function calling it. The relation of formats and dates is *very confused*.

The reason this screen works as it is is because in Drupal 7 you added formats and then paired formats with languages. So you surely had the right formats in this dropdown because it was coming from a pool of formats you entered. This actually works off of the main date format list now, which is confused. If I'd need to add the "Hungarian full date" as a separate high level format, that will show up alongside the "General full date" when selecting formats for date fields, display modes, etc. while we want the translations to kick in based on languages.

Let's imagine this screen would fulfill its promise and save those localizations somehow. So let's imagine we fix this page. Then, when you go back to the date format edit page, what would that show. Would it show it only applies to some languages and then the rest is edited on the localization page? That would not work for now either because the locale is not even removed from the format, so the next submission of the original form will overwrite your localizations.

Finally, also since #2098089: Date formats cannot be translated (the core UIs are useless) core now exposes the date format default configuration for community translation, so localize.drupal.org can feed the Drupal site with default date format localizations. That is amazing. Also the third built-in UI for localizing date formats due to that became the locale module UI. That actually allows to translate both the date format string and the date format label. But only for shipped date formats, not for new ones added on the UI. That works for the use case of free-form picking of date formats but is pretty darn hard to use, since there is no help (eg. precalculation of how the format would show like on the edit form) and the date format gets no context:

Now imagine you translated something here, how is that compatible with the other UIs? Well, it is not. The translation entered here will work so long as you don't resave the format on either of the other two screens above, which both will overwrite the translations (in different ways). So the two above UIs actually work against being able to use the community provided date format translations :/

Proposed resolution

Remove both date locale user interfaces and their underlying trickery. Let locale module handle the date format translation as it already does thanks to #2098089: Date formats cannot be translated (the core UIs are useless). As for a nicer user interface, #1952394: Add configuration translation user interface module in core includes a far superior user interface for date format translation (has free-form input, date format previews and allows translation of date formats + integrates with the localize.drupal.org pre-translation pulling nicely). No overwriting on one UI the data that was entered in a different UI. Also that UI is fully autogenerated based on config schema and configuration entity information, so almost no special code required to do it there. Only the date format preview needed special code. So we can remove lots of one-off code and get nicely generated screens that are better. What's not to love?

The config translation module already has this autogenerated listing UI for date formats (and other types of config entities similarly as well):

Once you pick a language on the translate page where the buttons lead, you can localize the format free-form with live preview and even translate the format name. And once again this works well with locale module's feeding of translations from localize.drupal.org too.

This translation page is also wired into the date formats listing page as a Translate dropdown operation so not needed to go to a different UI even to translate:

Remaining tasks

Review, commit.

User interface changes

Two useless interfaces removed.

API changes

None.

#1952394: Add configuration translation user interface module in core
#2098089: Date formats cannot be translated (the core UIs are useless)

Files: 
CommentFileSizeAuthor
#17 interdiff.txt1.2 KBGábor Hojtsy
#17 remove-bugos-conflicting-date-format-localization-17.patch30.45 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 59,977 pass(es). View
#15 interdiff.txt1.33 KBGábor Hojtsy
#15 remove-bugos-conflicting-date-format-localization-15.patch29.25 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 59,922 pass(es), 13 fail(s), and 5 exception(s). View
#12 remove-bugos-conflicting-date-format-localization-12.patch27.92 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 59,452 pass(es), 15 fail(s), and 5 exception(s). View
#12 interdiff.txt4.27 KBGábor Hojtsy
#11 interdiff.txt650 bytesGábor Hojtsy
#11 remove-bugos-conflicting-date-format-localization-11.patch23.65 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 60,355 pass(es), 35 fail(s), and 7 exception(s). View
#7 User interface translation | d8mi.localhost 2013-11-05 12-41-59.png114.31 KBGábor Hojtsy
#5 Date and time formats | d8mi.localhost 2013-11-05 11-59-02.png46.44 KBGábor Hojtsy
#4 remove-bugos-conflicting-date-format-localization.patch23.33 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 59,334 pass(es), 56 fail(s), and 15 exception(s). View
#3 Edit Hungarian translation for Default long date dátumformátum | d8mi.localhost 2013-11-05 11-40-31.png59.87 KBGábor Hojtsy
#3 Dátumformátum | d8mi.localhost 2013-11-05 11-39-13.png79.88 KBGábor Hojtsy
#1 2013-11-05 11-03-26.png179.87 KBGábor Hojtsy
Localize | sb289d9df7618ea6.s2.simplytest.me 2013-11-05 10-58-03.png41.23 KBGábor Hojtsy
Edit date format | sb289d9df7618ea6.s2.simplytest.me 2013-11-05 10-57-20.png76.01 KBGábor Hojtsy
Date and time formats | sb289d9df7618ea6.s2.simplytest.me 2013-11-05 10-54-41.png64.97 KBGábor Hojtsy

Comments

Gábor Hojtsy’s picture

Issue summary: View changes
Issue tags: +D8MI, +language-config, +sprint
FileSize
179.87 KB
Gábor Hojtsy’s picture

Title: Remove bugos and duplicate date languages UIs » Remove two bugos and duplicate date languages UIs
Issue summary: View changes
Gábor Hojtsy’s picture

Title: Remove two bugos and duplicate date languages UIs » Remove two bogus and duplicate date languages UIs
Status: Active » Needs review
FileSize
23.33 KB
FAILED: [[SimpleTest]]: [MySQL] 59,334 pass(es), 56 fail(s), and 15 exception(s). View

Diffstat not bad, ha :D

 b/core/modules/system/config/schema/system.schema.yml                             |    6 
 b/core/modules/system/config/system.date_format.fallback.yml                      |    1 
 b/core/modules/system/config/system.date_format.html_date.yml                     |    1 
 b/core/modules/system/config/system.date_format.html_datetime.yml                 |    1 
 b/core/modules/system/config/system.date_format.html_month.yml                    |    1 
 b/core/modules/system/config/system.date_format.html_time.yml                     |    1 
 b/core/modules/system/config/system.date_format.html_week.yml                     |    1 
 b/core/modules/system/config/system.date_format.html_year.yml                     |    1 
 b/core/modules/system/config/system.date_format.html_yearless_date.yml            |    1 
 b/core/modules/system/config/system.date_format.long.yml                          |    1 
 b/core/modules/system/config/system.date_format.medium.yml                        |    1 
 b/core/modules/system/config/system.date_format.short.yml                         |    1 
 b/core/modules/system/lib/Drupal/system/DateFormatInterface.php                   |   38 --
 b/core/modules/system/lib/Drupal/system/Entity/DateFormat.php                     |   76 -----
 b/core/modules/system/lib/Drupal/system/Form/DateFormatFormBase.php               |   22 -
 b/core/modules/system/system.admin.inc                                            |  137 ----------
 b/core/modules/system/system.module                                               |   23 -
 b/core/modules/system/system.routing.yml                                          |   22 -
 core/modules/system/lib/Drupal/system/Controller/DateFormatLanguageController.php |   51 ---
 core/modules/system/lib/Drupal/system/Form/DateFormatLocalizeResetForm.php        |  117 --------
 core/modules/system/lib/Drupal/system/Form/SystemForm.php                         |   24 -
 21 files changed, 6 insertions(+), 521 deletions(-)
Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Title: Remove two bogus and duplicate date languages UIs » Remove two (out of 3) bogus and duplicate date languages UIs
Issue summary: View changes
FileSize
114.31 KB

Status: Needs review » Needs work

The last submitted patch, 4: remove-bugos-conflicting-date-format-localization.patch, failed testing.

Gábor Hojtsy’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

FileSize
23.65 KB
FAILED: [[SimpleTest]]: [MySQL] 60,355 pass(es), 35 fail(s), and 7 exception(s). View
650 bytes

Use proper Language class solves most of the fails. Now for the actual fail.

Gábor Hojtsy’s picture

FileSize
4.27 KB
27.92 KB
FAILED: [[SimpleTest]]: [MySQL] 59,452 pass(es), 15 fail(s), and 5 exception(s). View

I looked into solving the fail in DateFormatsLanguageTest.php but the tests added in #2098089: Date formats cannot be translated (the core UIs are useless) cover this use case pretty well. The test in DateFormatsLanguageTest.php is amazingly broad, it attempts to prove that a date format can be translated by looking at a node's display page and the date displayed on that page. The tests added in #2098089: Date formats cannot be translated (the core UIs are useless) actually test the API and the locale storage of the translation.

Note that this test does explain somewhat of the flawed thinking of the date format localization approach of this system. You need to add extra formats to associate as translations of other formats. It also demonstrates it works under some conditions, but "I want to localize format A, let's create format B, set A not to apply to the desired language and then go to a different UI to associate format B with format A and then see both A and B show up as separate formats where date formatting is offered" is a multi-level flawed approach.

Status: Needs review » Needs work

The last submitted patch, 12: remove-bugos-conflicting-date-format-localization-12.patch, failed testing.

andypost’s picture

Re-grouping tests is needed anyway, suppose better to file follow-up to clean-up tests by separating them on unut and we tests

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
29.25 KB
FAILED: [[SimpleTest]]: [MySQL] 59,922 pass(es), 13 fail(s), and 5 exception(s). View
1.33 KB

@andypost I'm not sure what you are referring to, the tests being removed are already covered by the tests added in #2098089: Date formats cannot be translated (the core UIs are useless). Also found this test method (that was failing), which had this issue. Already covered there as well. In fact that test uses the typed config system to verify this :)

Status: Needs review » Needs work

The last submitted patch, 15: remove-bugos-conflicting-date-format-localization-15.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
30.45 KB
PASSED: [[SimpleTest]]: [MySQL] 59,977 pass(es). View
1.2 KB

The remaining fail is related to language module defining a local task for a route that system module provides unconditionally anyway. Pretty twisted... Why would that not be defined by system module?! Oh, I guess so the page looks like its not there?! Well. Anyway, needs to go, since the code backing that up is not there anymore either. Passes with the only remaining failed test for me, so let's see!

Gábor Hojtsy’s picture

Also a DX improvement since it unifies how date formats are translated / localized with how any other config entity (or config for that matter). It is also a UX improvement since it lets free form date format entry with previews for translations, integrates with localize.drupal.org well and uses the same UI affordances as translating any other configuration (eg. node type, view, field, etc).

cosmicdreams’s picture

As one of the developers that touched all of this code when getting date into Drupal 8 I'll give a deeper review on this tonight. My initial impression is that this looks good.

cosmicdreams’s picture

Issue summary: View changes
cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

Gábor Hojtsy’s picture

Issue tags: +Spark

Worked on this as part of Spark assignments, so tagging with Spark as well.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Thanks a lot for all the detail here.

It sounds like this patch allows us to safely remove several hundred lines of code while still preserving the ability to translate this stuff as we wait for the penultimate solution to materialize. It was RTBCed by someone who helped in adding some of those (now removed) UIs to core. Sounds good to me!

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -sprint

Yay, thanks!

Status: Fixed » Closed (fixed)

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