- 🇨🇳China jungle Chongqing, China
First, posting the raw interdiff content of 0 bytes
interdiff-32-33.txt
$ diff 2381147-32.patch 2381147-33.patch 83c83 < index 42d54b21ef..d1d7bad78f 100644 --- > index f60c0a56ee..fc053f29f8 100644 94c94 < @@ -1108,6 +1109,70 @@ public function testMenuTranslationWithoutChange() { --- > @@ -1111,6 +1112,70 @@ public function testMenuTranslationWithoutChange() { 191c191 < index 08bc9aad87..5a9f3ac4f4 100644 --- > index e0fa5d3d83..ae2ab6ee2f 100644 194,195c194,195 < @@ -21,15 +21,8 @@ field.field_settings.text: < label: 'Text (formatted) settings' --- > @@ -27,15 +27,8 @@ field.field_settings.text: > type: string 211,212c211,212 < @@ -40,15 +33,8 @@ field.field_settings.text_long: < type: mapping --- > @@ -52,15 +45,8 @@ field.field_settings.text_long: > type: string 228,229c228,229 < @@ -66,18 +52,8 @@ field.field_settings.text_with_summary: < label: 'Require summary' --- > @@ -83,18 +69,8 @@ field.field_settings.text_with_summary: > type: string
- 🇨🇳China jungle Chongqing, China
2. review the schema changes
Posting the schema of text_format from
core/config/schema/core.data_types.schema.yml
# Text with a text format.
text_format:
type: mapping
label: 'Text with text format'
# We declare the entire mapping of text and text format as translatable. This
# causes the entire mapping to be saved to the language overrides of the
# configuration. Storing only the (to be formatted) text could result in
# security problems in case the text format of the source text is changed.
translatable: true
mapping:
value:
type: text
label: 'Text'
# Mark the actual text as translatable (in addition to the entire mapping
# being marked as translatable) so that shipped configuration with
# formatted text can participate in the string translation system.
translatable: true
format:
type: string
label: 'Text format'
# The text format should not be translated as part of the string
# translation system, so this is not marked as translatable.+++ b/core/modules/text/config/schema/text.data_types.schema.yml @@ -0,0 +1,19 @@ +# See the 'text_format' type in core.data_types.schema.yml. +text_format_with_summary: + label: 'Default' + type: mapping + translatable: true + # Integrate with the Configuration Translation module. + form_element_class: 'Drupal\text\FormElement\TextFormatWithSummary' ... + value: + type: text + label: 'Body' + translatable: true + summary: + type: string + label: 'Summary' + translatable: true + format: + type: string + label: 'Text format'
- With the
form_element_class
key which is good, seehook_config_schema_info_alter
below - format is not translatable as text_format
function hook_config_schema_info_alter(&$definitions) { // Enhance the text and date type definitions with classes to generate proper // form elements in ConfigTranslationFormBase. Other translatable types will // appear as a one line textfield. $definitions['text']['form_element_class'] = '\Drupal\config_translation\FormElement\Textarea'; $definitions['date_format']['form_element_class'] = '\Drupal\config_translation\FormElement\DateFormat'; }
- With the
- Status changed to Needs work
almost 2 years ago 9:24pm 23 February 2023 - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
+++ b/core/modules/config_translation/src/FormElement/TextFormat.php @@ -27,14 +28,15 @@ public function getSourceElement(LanguageInterface $source_language, $source_con - return [ + return NestedArray::mergeDeep(parent::getTranslationElement($translation_language, $source_config, $translation_config), [ ... - ] + parent::getTranslationElement($translation_language, $source_config, $translation_config);
Should we respect the order here? I'm not sure if it's relevant in this case, but could hide bugs.
PHP arrays addition is not commutative (A+B != B+A).I don't see why those css classes are needed now and not before, but being fair I didn't really test it.
+++ b/core/modules/text/config/schema/text.schema.yml @@ -27,15 +27,8 @@ field.field_settings.text: field.value.text: - type: mapping + type: text_format label: 'Default value' - mapping: - value: - type: label - label: 'Value' - format: - type: string - label: 'Text format'
This is great!
- 🇨🇳China jungle Chongqing, China
+++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php @@ -1111,6 +1112,70 @@ public function testMenuTranslationWithoutChange() { + * Test translation of a default value of a text with summary field.
As it set back to NW, here should start with 'Tests', #3133162: Replace the start verb Test with Tests in method comments of tests →
I am trying to test it manually, but I can not get the same result as the test. Leaving this issue temporarily
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
-
+++ b/core/modules/text/config/schema/text.data_types.schema.yml @@ -0,0 +1,19 @@ +# See the 'text_format' type in core.data_types.schema.yml.
While we're making this consistent, we should move the
text_format
type out ofcore.data_types.schema.yml
and into this file. -
+++ b/core/modules/text/src/FormElement/TextFormatWithSummary.php @@ -0,0 +1,41 @@ +use Drupal\config_translation\FormElement\TextFormat;
Let's move this class into the
text
module too while we're at it?
-
- Merge request !5023Issue #2381147: Text and text with summary field default value config does not use the text_format schema type → (Open) created by smustgrave
- last update
about 1 year ago 30,388 pass, 10 fail - last update
about 1 year ago 30,388 pass, 10 fail - 🇺🇸United States smustgrave
Sorry for taking so long to get back to this.
#39
Should we respect the order here? I'm not sure if it's relevant in this case, but could hide bugs.
that I'm not 100% about, may need a bigger brain then my own
#40 fixed
#41.1 moved
#41.2 movedMoved to MR so hiding patches
- last update
about 1 year ago 30,403 pass, 2 fail - last update
about 1 year ago 30,403 pass, 2 fail - last update
about 1 year ago 30,403 pass, 2 fail - last update
about 1 year ago 30,403 pass, 2 fail - last update
about 1 year ago 30,415 pass - Status changed to Needs review
about 1 year ago 10:20pm 16 October 2023 - 🇺🇸United States smustgrave
Reverted #41.1 as it caused some failures and may want to handle in a follow up.
- Status changed to RTBC
about 1 year ago 8:19am 22 October 2023 - last update
about 1 year ago 30,428 pass - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
Created 📌 Move TextFormat form element from config_translation to text module Active
Created 📌 Move text_format data type from core to text module ActiveI think they are independent of each other. Removing "needs followup" tag.
- Status changed to Needs review
about 1 year ago 8:42am 23 October 2023 - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 30,429 pass - Status changed to RTBC
about 1 year ago 11:42am 24 October 2023 - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
We have split the bits that require BC layer into follow-ups. The fix itself was RTBC before, and I think it is, so setting it back to RTBC.
- last update
about 1 year ago 30,439 pass - last update
about 1 year ago 30,444 pass - last update
about 1 year ago 30,466 pass - last update
about 1 year ago 30,485 pass - last update
about 1 year ago 30,487 pass - last update
about 1 year ago 30,488 pass - last update
about 1 year ago 30,490 pass 5:21 2:19 Running- last update
about 1 year ago 30,518 pass - last update
about 1 year ago 30,530 pass - last update
about 1 year ago 30,553 pass - last update
about 1 year ago 30,562 pass - last update
about 1 year ago 30,604 pass - Status changed to Needs work
about 1 year ago 9:03am 20 November 2023 - 🇳🇿New Zealand quietone
I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions or other work to do.
The diff does not apply to 11.x, there setting to NW for a rebase.
This may need a CR with a screenshot informing the user of this new capability.
- First commit to issue fork.
- Status changed to Needs review
about 1 year ago 10:35am 20 November 2023 - 🇮🇳India ankithashetty Karnataka, India
Rebased done, hence moving to NR.
Still need a update on CR as requested in #52.Thanks!
- Status changed to RTBC
about 1 year ago 12:48pm 20 November 2023 - last update
about 1 year ago 30,607 pass - last update
about 1 year ago 30,608 pass - last update
about 1 year ago 30,670 pass - last update
about 1 year ago 30,677 pass - last update
about 1 year ago 30,686 pass - last update
about 1 year ago 30,690 pass - last update
about 1 year ago 30,690 pass - last update
about 1 year ago 30,690 pass - last update
about 1 year ago 30,698 pass - last update
about 1 year ago 30,700 pass - last update
about 1 year ago 30,714 pass - last update
about 1 year ago 30,726 pass - last update
about 1 year ago 30,766 pass - last update
about 1 year ago 30,768 pass - Status changed to Needs work
about 1 year ago 9:08am 19 December 2023 - 🇳🇿New Zealand quietone
Always nice to see an old issue getting attention, thanks!
Comment #53 states that this is in need of a CR, therefor setting back to needs work.
To learn more about what this is fixing I tested on Drupal 11.x today, standard install, with a second language and a Textwithsummary field. I was able to translate the default value field without the patch here. Yet, the issue summary states that this change is to allow, "Default values of text fields can be translated using a text editor." Am I missing something? I don't work with translations so maybe I am.
I applied that patch and the translation modal was changed so that both the english default value and the translation default value can be edited. I don't think that it correct, only the translation should be editable. In fact the title is "Edit Italian translation for TextWithSummary" so it should only be for the Italian version.
It is not clear to me what is being fixed here. Tagging for an issue summary update. And since this is changing the UI it should have links to up to date before/after screenshots in the issue summary.
Looking back at the comments I do not see any manual testing. I am adding a tag for that.