Entity forms will let users edit different language variants of the same piece of data. Actually only one language variant will be editable at any time so we need a reliable way to know which is the form "active" language. Simply relying on the current content language does not always work as it sometimes makes pretty unconfortable the editing workflow, since the user may want to keep her current language and edit different language variants. Hence we need to be able to explicitly set the active form language through a parameter.
Comment | File | Size | Author |
---|---|---|---|
#44 | et-form_language-1498874-44.patch | 12.22 KB | Gábor Hojtsy |
#41 | et-form_language-1498874-41.patch | 13.14 KB | Gábor Hojtsy |
#34 | et-form_language-1498874-34.patch | 11.65 KB | plach |
#31 | et-form_language-1498874-31.interdiff.do_not_test.patch | 784 bytes | plach |
#31 | et-form_language-1498874-31.patch | 11.69 KB | plach |
Comments
Comment #0.0
plachUpdated issue summary.
Comment #0.1
plachUpdated issue summary.
Comment #1
plachThis also concerns multilingual configuration.
Related issue: #1498880: Theme language switcher for seven theme.
Comment #2
andypostthere's a mention about forms in #1448330-25: [META] Discuss internationalization of configuration
Symfony has a great formAPI
Also I think the langcode properties/fields affects mostly on render layer of core
Comment #3
plachMarking as a follow up of #1499596: Introduce a basic entity form controller.
Comment #4
Gábor Hojtsy@plach: can you provide an updated version of your thinking on this. I got @vasi1186 to work on #1280996: New language_select element type for form API for language selection generalization. Would be good to find common grounds sooner than later.
Comment #5
peximo CreditAttribution: peximo commentedHere is a first attempt. The patch introduces a new entity form language type and uses it as a default value for the form language if no one is explictly provided to
entity_get_form()
.Comment #6
Gábor HojtsyComment #8
peximo CreditAttribution: peximo commentedFixed negotiation callback for new language type and some tests.
Comment #9
fagoAs discussed with plach we think it makes more sense to have general language type instead of this one. E.g. language_content_edit or something like that, and then use this as default. Let's do so then.
Comment #10
attiks CreditAttribution: attiks commentedrenamed
Comment #11
plach@fago and I agreed to move the form language type to the language module. Actually @peximo has already a patch for this but apparently he did not post it...
Comment #12
Gábor Hojtsy@plach: that sounds good, makes sense.
Comment #13
plachHowever Jose is not convinced that this approach can work for multiple realms, so I think we need to find an alternative not involving a new langauge type. Probably just relying on the content language as a default and relying on Request parameters otherwise.
Comment #14
peximo CreditAttribution: peximo commentedI'll work as per #13 on an alternative version without a new language type.
Comment #15
peximo CreditAttribution: peximo commentedRemoved the new language_form language type and set language_content as form default language.
Comment #16
fagoGoing without a separate language for now is fine to move on I think. As discussed let's keep the default in $form_state['langcode'] initially so the form controller can deal with it properly.
Comment #17
plachAddressed #15 since @peximo is working on other stuff, right now.
Comment #18
fagoOh, that looks very clean now :)
Comment and code doesn't do the same? It uses content language and applies language fallbacks. We should do a final fallback to default entity language though if none of the fallback-languages is in the entity, or?
Comment #19
peximo CreditAttribution: peximo commentedComments fixed as per fago's suggestion. Actually we already fall back to the entity language.
Comment #20
plachTalked with @fago and he's ok with the latest patch.
Comment #21
peximo CreditAttribution: peximo commentedgetFormLangcode() should return a langcode not a language object. Fixed.
Comment #23
plachWe need tests for this, moreover the fix won't work if Language module is disabled. We need to fix the
Entity::language()
method to always return a valid language object.Comment #24
Gábor Hojtsy@peximo: are you continuing work on this issue? It is a very important part of making Drupal 8 multilingual! Thanks for working on it so far!
Comment #25
peximo CreditAttribution: peximo commented@gabor: I'll work on this issue tomorrow morning.
Comment #26
peximo CreditAttribution: peximo commentedFixed Entity::language() method and added some tests.
@gabor: plach thinks is better to do the test with the entity_test form controller (see #1757232: Improve test coverage for the entity form controller). So we might need to merge the two issues.
Comment #28
plachSorry, we were not taking entity language changes into consideration. Rerolled the patch with that fixed plus a couple of minor adjustments.
Comment #29
plachRetitling and untagging since we are not trying to handle form language in a generic fashion as per the discussion with @Jose.
Comment #29.0
plachUpdated issue summary.
Comment #31
plachActually there may be cases where the entity langcode is not set, hence notices are thrown.
Comment #33
plachNo failures on my box.
#31: et-form_language-1498874-31.patch queued for re-testing.
Comment #34
plachRerolled after #1761040: Rename Storable, Entity, and Configurable to Entity, ContentEntity, and ConfigEntity landed.
Comment #35
sunI'm not sure I fully understand why this is necessary (issue summary is a bit sparse), but all of you seem to agree that it is a good idea. I hope you guys asked each other some criticizing questions in the discussions ;)
The code in EntityFormController::getFormLangcode() could be improved for monolingual sites and clarified in terms of code logic, but that is pretty minor and can be done in a later follow-up.
Comment #36
Gábor HojtsyThis deals with the rapidly changing entity landscape and is at a definite risk of commit conflicts.
Comment #37
Gábor Hojtsy#34: et-form_language-1498874-34.patch queued for re-testing.
Comment #38
catchThis looks great. just one question:
Why not adding multilingual forms to the entity_test module?
Comment #39
Gábor Hojtsy@catch: the entity_test module is a pure API test module, it does not have page callbacks or forms at all. I think it would be important at this point to get this moving instead of pushing it back to implement a UI for testing in entity_test module while we already have the necessary stuff to test elsewhere, such as in node module.
Comment #40
catchOK that's fine, but in that case let's add a new test class for the forms, so we don't need to install node module for the other test methods.
Comment #41
Gábor HojtsyMoved it to a separate file with only the setup routines required for this one.
Comment #42
Gábor HojtsyShould be back to RTBC then.
Comment #43
catchThis change shouldn't be needed then?
Comment #44
Gábor HojtsyIndeed, we should not have any business touching that file anymore.
Comment #45
catchThanks! Committed/pushed to 8.x.
I don't think this really needs change notice so moving straight to fixed, but please re-open/write one if you disagree.
Comment #46
plachI think we don't need a change notice here: this ended-up being a far less APIsh change than I originally thought.
Comment #47
Gábor HojtsyThis should be done for our sprint then :) Thanks all!
Comment #49
chx CreditAttribution: chx commentedComment #49.0
chx CreditAttribution: chx commentedUpdated issue summary.