Problem/Motivation
Config translation has a schema type for text elements with format. However views area plugin "Text" does not use this schema type.
Proposed resolution
Use the new text_format type. Restructure the views data structure to use that nested data format.
Remaining tasks
Commit.
Steps to test
- apply patch
- if not committed yet, apply patch from #2144413: Config translation does not support text elements with a format
- install
- enable modules: config_translation (which will enable language)
- add a language, admin/config/regional/language
- go to a view, content list, or create a new testing view.
- Add to the Header, a global: text area. You might want to check off: display even if no results.
- Add to the Footer, a global: test area. You might want to check off: display even if no results.
- translate the view. Expand out lots of stuff till you find the header.
- Notice that the format text area widget is used. Nice.
- (not part of this issue?) change the text. save. view the view in both default language and added language, see if header text changes.
User interface changes
Use appropriate text format widget for editing translatable part of config. WYSIWYG will appear in config translation when translating this type of field. The source will appear in a disabled WYSIWYG also.
API changes
View structure changes to use the nested text + format.
Comments
Comment #1
webflo CreditAttribution: webflo commentedComment #2
Gábor HojtsyThere are other elements like footer, etc here possibly no?
Comment #3
vijaycs85isn't it [content][contains][value]?
isn't it [content][contains][format]?
Comment #4
webflo CreditAttribution: webflo commentedI don´t think so. The 'contains' key is a special key that is used in PluginBase::setOptionDefaults to apply default values in nested option arrays.
Comment #5
dawehnerThe patch is right. contains is just used to set the default values, nowhere else.
Comment #6
YesCT CreditAttribution: YesCT commentedsteps to test.
Comment #7
YesCT CreditAttribution: YesCT commentedoops dont need the test module
Comment #8
YesCT CreditAttribution: YesCT commentedwhy are we adding this, specifically for the header?
I tried editing a view, and header is not text format by default, have to add global: text.
that can also be added to footer, and a field.
Comment #9
YesCT CreditAttribution: YesCT commentedupdated steps to test.
should saving work?
Translating the header text results in that new text in the default config/language for me.
After patches:
Here is what the translate view looks like which uses the text format widget thing.
Before patches:
... header does not show up in the translate at all.
no screenshot.
So, maybe there is another issue here, just getting the header (footer, other) text to show up in the config translation at all. Probably just needs to be added to the schema.
Then, also to use the new text format type.
This issue is fixing both issues in one.
Needs work (but leaving it postponed):
- translation overwrites untranslated config
- need to add schema for footer
- need to add schema for fields (which might be global text area). This might be covered as part of another issue general views field schema, or general views translation ui.
Comment #10
YesCT CreditAttribution: YesCT commentednote the views edit of header does not use the fancy widget. So.. maybe config translate should not either?
Comment #11
dawehnerJust in case someone wonders why we don't use a FULL wysiwyg editor in the views header:
These have caused troubles when they were loading in dialogs previously, WARNING! #1065344: No text area for header/footer/empty text when creating view !WARNING
Comment #12
Gábor HojtsyRemoving from sprint since this was postponed for so long....
Comment #13
tstoecklerI just tried enabling editor support in View UI, but it still does not work in D8. If you try to insert an image. The image dialog replaces the Views UI overlay-dialog, and then pressing save in the image dialog does not work.
Comment #14
tstoecklerOpened #2272369: What to do with nested dialogs? for that problem. I think we should proceed here without #2144413: Config translation does not support text elements with a format in the meantime. I.e. we should adapt the config structure to have a 'value' and 'format' key as is it were using 'type: text_format' in the schema, but simply use two 'type: string's for now. Then once the referenced issue is in, we can switch to 'type: text_format' without breaking anything, i.e. past beta.
Comment #15
tstoecklerComment #16
Gábor Hojtsy@tstoeckler: it is already two distinct items, not sure what is to do here outside of adapting to #2144413: Config translation does not support text elements with a format. Besides, I think the goal for beta is to have a stable data model. If we know we want to make that change later, then beta is not that stable... So it would be important for us to complete this before, although not vital. We started on it in Prague. How can I help to make #2144413: Config translation does not support text elements with a format happen?
Comment #17
tstoecklerWell, I meant changes like this actually. We can actually make those without using the 'text_format' schema type for now.
Comment #18
Gábor HojtsyIs that using a structure and keys that #2144413: Config translation does not support text elements with a format would work with? Are we sure we agreed on #2144413: Config translation does not support text elements with a format enough to make these changes now?
Comment #19
tstoecklerWell @Jose Reyero recently voiced some concerns over there, but I didn't see anything related to the config structure. (Only where the translatable: true sits). All patches so far have used the same config structure and so far no one has complained. So I'd tentatively, say "Yes?!".
Comment #20
Gábor HojtsyLet's move forward here then.
Comment #21
Gábor HojtsyComment #22
Gábor Hojtsy#2144413: Config translation does not support text elements with a format landed. Moving this to the sprint.
Comment #23
vijaycs85Initial patch...
found few regressions and filed them at #2381067: Regression: Config translation's text_format doesn't display the text in view area
Comment #24
Gábor Hojtsy@vijaycs85: The issue you opened seems to be a bug with how you introduced text format in your patch, not in head. Look at the original patch above by @webflo and #2144413: Config translation does not support text elements with a format. Basically text formatting is supported on a nested mapping level (format and value are on the same level but nothing else). It is not enough to just use a different format in these views places, their structure needs to change, which is what @webflo has provisions for above. Your patch just changes the types, but that is what leads to the bugs you reported at #2381067: Regression: Config translation's text_format doesn't display the text in view area in the first place.
The text format type definition is:
Just changing a text type to text_format will not suffice. It makes a string into a mapping :D
Comment #26
tstoecklerYes, the patch in #1 was the right approach. Reviving that as it didn't apply anymore. (Couldn't be bothered to re-roll properly, just re-created from scratch, thus no interdiff.)
I checked and the
format_key
stuff is obsolete already (i.e. used nowhere in core) and it is a leftover of the locale integration of Views in D7.Comment #27
Gábor HojtsyThanks, spotted one issue:
Should not have the standalone format key (at the bottom of this snippet) anymore, right?
Also @vijaycs85 applied the format to other elements as well, not just the text type.
Comment #29
vijaycs85- oops, sorry for the wrong alarm.
Comment #30
tstoecklerRe #27:
Oops, yes, forgot to delete that.
I saw that in #23 but I don't think it's correct. There's a couple area handlers, but one is just plain, non-formatted text (
text_custom
) and one shows the number of results, etc. Only thetext
one actually uses'#type' => 'processed_text'
(i.e.check_markup()
), though, so I think #26 is correct. Please do go and check that I'm not on crack, though! :-)Btw. I grepped all config schemas for "format" and in the process I found #2381147: Text and text with summary field default value config does not use the text_format schema type
That issue is a little bit more tricky (because of the text_with_summary thing...) but that should cover it. I don't think we have anything using formatted text that does not use the 'format' string.
Comment #31
tstoecklerWorking on this.
Comment #32
tstoecklerThis should be green.
Comment #33
Gábor HojtsyThe changes all make sense. So I think what this should do in practice is config translation module should display "No front page content has been created yet." in a WYSIWYG widget on the translation UI right? I cannot reproduce that unfortunately. What am I missing?
Comment #34
Gábor HojtsyUhm, that is "area_text_custom" type, not to expect to have WYSIWYG on it. Duh.
Comment #35
Gábor HojtsySo I added a text field, and it works flawlessly, so this does not only look good in code, it works well in practice also. :)
Comment #36
Gábor HojtsyBTW the views UI itself does not even embed the WYSIWYG for the same setting (it only has the good old text format selection dropdown) so the config translation UI is easier to use for this field :O
Comment #37
Gábor HojtsyChanges view formatted text field structure, so tagging accordingly.
Comment #38
tstoeckler:-) Yeah, Views UI explicitly disables the text editor as loading a text editor in a popup is not safe ATM because the editor itself can open dialogs (i.e. for inserting an image) and loading a dialog inside of a dialog currently goes massively wrong.
Comment #39
tstoecklerSee also #2272369: What to do with nested dialogs? for that...
Comment #40
tstoecklerSorry, #fail. That was already in the related issues....
Comment #41
dawehnerThis looks certainly good so far!
Can't we drop the options 'format' then?
Comment #42
tstoecklerOh right. Had totally forgotten about that, sorry. Thanks for noticing!
Leaving at RTBC.
Comment #43
Gábor Hojtsy@dawehner: haha, I had the same question in #27, don't know how I missed that it was not fixed. Thanks @tstoeckler for fixing, I agree keeping RTBC.
Comment #44
Gábor HojtsyUpdated issue summary.
Comment #45
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 48813a2 and pushed to 8.0.x. Thanks!
Comment #46
Gábor HojtsyYay thanks, now onto #2381147: Text and text with summary field default value config does not use the text_format schema type :)