On 7.x it is there, but has the default of visible (incovenient, needs to be hidden for any sites I do).
Disappeared in 8.x.
Now I dont see the need for it in the display tab, but perhaps there is a use case where one might want to display the language on a node. SO lets re-add it
Gabor noted in #1074672:
indeed it not appearing on the manage display screen at all seems like (a separate) regression, that would need its own tests too. It only applies to Drupal 8. However, it sounds like an improvement that nodes would not have the language field displayed automatically anymore.
So, opening a new issue, I hope to work on it later this week :-)
Comment | File | Size | Author |
---|---|---|---|
#41 | node_1757504_41.patch | 2.67 KB | maxtorete |
#40 | node_1757504_40.patch | 2.68 KB | maxtorete |
#36 | node_1757504_36.patch | 6.68 KB | maxtorete |
#34 | node_1757504_34.patch | 6.68 KB | maxtorete |
#30 | node_1757504_30.patch | 6.36 KB | maxtorete |
Comments
Comment #1
Gábor HojtsyTagging up, cleaning title.
Comment #2
boran CreditAttribution: boran commentedby adding the following in node.module/node_field_extra_fields() after $extra['node'][$bundle->type]['form']['language'], the language will appear in the display tab.
But it defaults to visible, where as it needs to be hidden.
Looked at core/modules/field/field.api.php and core/modules/field_ui/field_ui.api.php, but its not obvious to me what hook needs to be called set set the default visible of the language in "manage display".
Any suggestions?
Comment #3
Gábor Hojtsy@boran: thanks for the info! Is this the same code that was present in Drupal 7? Did you manage to dig up any reasons as to why it was removed in Drupal 8?
Comment #4
Gábor HojtsyWill also need tests. If there would have been tests for this, we would not have lost this.
Comment #5
boran CreditAttribution: boran commentedCode and test attached.
Still one problem: how to set the default value of Language on the display tab to "hidden"?
D7: the i18n module handles this in D7.
Comment #7
boran CreditAttribution: boran commented"The testbot client is probably malfunctioning." :-)
Comment #8
Gábor Hojtsy#5: node_1757504_5_code.patch queued for re-testing.
Comment #9
Gábor Hojtsy#5: node_1757504_5_test.patch queued for re-testing.
Comment #11
boran CreditAttribution: boran commentedI'm very new to tests, any suggestions on why the test fails? Why does for example FileFieldWidgetTest complain?
Also looking suggestions for
with a defautl of hidden the patch should not be committed, as it disturb the UI experience (will have to be manually set to hidden on most sites).
Comment #12
maxtorete CreditAttribution: maxtorete commentedI have made a patch to fix this issue. To achieve it, I have made these changes:
Comment #13
maxtorete CreditAttribution: maxtorete commentedThis is the same patch as #12, but I trimmed some white spaces on blank line.
Comment #15
maxtorete CreditAttribution: maxtorete commented#13: node_1757504_13.patch queued for re-testing.
Comment #17
maxtorete CreditAttribution: maxtorete commentedAs said on #7, testbot is making some strange things... I attach a new patch (comment too long splitted on two lines).
Comment #18
maxtorete CreditAttribution: maxtorete commentedI forgot to delete a useless line. Added patch with that correction.
Comment #19
c31ck CreditAttribution: c31ck commentedThis looks really good. The patch applies cleanly, the language field is shown on the manage display tab and it is hidden by default. However, when configuring the language to display and actually viewing the node, the language is not displayed. Do we need to make changes to node_build_content() to make this work?
Shouldn't this be $this->assert(!empty($language_display) instead of $this->assert(!empty($language_field)?
Whitespace issue.
I'm not sure if the 'Hide language selector' option should also control the display of the language field when viewing a node. I think it should only have an effect on the 'manage fields' tab and not on the 'manage display' tab.
Comment #20
maxtorete CreditAttribution: maxtorete commentedI think you're right, I have fixed this and the other issues you noticed. Thanks!
Comment #22
maxtorete CreditAttribution: maxtorete commented#20: node_1757504_20.patch queued for re-testing.
Comment #24
maxtorete CreditAttribution: maxtorete commentedThis could be a random issue on testbot (reported issue #1779638: Unexplained test failure in LocaleFileImportStatus->testBulkImportUpdateExisting()), locally it passes all tests. I'm sending it again to test.
Comment #25
maxtorete CreditAttribution: maxtorete commentedI'm not sure if we should add it on translation_node_view on translation module or add it on node_view function on node module.
Comment #26
maxtorete CreditAttribution: maxtorete commentedAs the extra field for language info is added on node module, I have implemented it on node_view.
Comment #27
maxtorete CreditAttribution: maxtorete commentedComment without ending dot fixed.
Comment #28
Gábor HojtsyThese tests look good, but can we expand a bit on them to test that the Language field can be shown if we want to.
Comment #29
maxtorete CreditAttribution: maxtorete commentedOk, I'm on it.
Comment #30
maxtorete CreditAttribution: maxtorete commentedI have created NodeTypeInitialLanguageTest->testLanguageFieldVisibility() to check if the Language field can be unhidden and it is shown after that.
I thought to create testLanguageFieldVisibility on NodeFieldMultilingualTestCase, but it is Field API related so I think that it doesn't fit there.
Comment #31
maxtorete CreditAttribution: maxtorete commentedComment #32
Gábor HojtsyNew test looking very good! Only one more thing from me:
Can we create the node first, check that the field is not visible and then set it to visible and check that it became visible? Sounds like a little change for a much more comprehensive test.
Comment #33
maxtorete CreditAttribution: maxtorete commentedGábor you read my mind, I was making now that change!
Comment #34
maxtorete CreditAttribution: maxtorete commentedNew patch attached with default settings test in NodeTypeInitialLanguageTest->testLanguageFieldVisibility().
Comment #35
maxtorete CreditAttribution: maxtorete commentedComment #36
maxtorete CreditAttribution: maxtorete commentedChanged assert to assertTrue.
Comment #37
c31ck CreditAttribution: c31ck commentedTest now includes the recommendations from #32. This looks good to go to me.
Comment #38
webchick(nitpick) "context" should be indented so it's aligned with "visible". Fixed in commit.
No t() on assertions. Fixed in commit.
I stared and stared at this and cannot figure out why it works, but I did confirm that it does by local testing. :) (And of course the test coverage confirms it, too.) I expected to see a #access or a #type switch or something. Huh.
Anyway, committed and pushed to 8.x. Thanks!
As a side-note, I uncovered #1783352: Dragging an item to/from the visible/hidden portion of Field UI does not make it visible/hidden. while testing this patch. Nothing to do with D8MI though. :)
Comment #39
maxtorete CreditAttribution: maxtorete commentedWell, I think that we should add the field if Language module is enabled, and then it will be hidden when the node is rendered, but the Language field is still accesible to hooks or templates.
Now, I think I should have added the Language field on node_build_content so it could be manipulated on hook_node_view and hook_entity_view implementations. I have the patch to this change, should I add a new issue or attach it here and change the status to need review?
Comment #40
maxtorete CreditAttribution: maxtorete commentedAs I have talked with Gábor Hojtsy, we should show the Language field on manage display even if Language module is not enabled.
I also have moved the field addition to node_build_content so it could be manipulated on hook_node_view and hook_entity_view implementations.
Comment #41
maxtorete CreditAttribution: maxtorete commentedOld comment updated.
Comment #42
c31ck CreditAttribution: c31ck commentedThis looks good, patch applies cleanly and works as expected. Moving this to node_build_content and making the language field available regardless of language module being active make perfect sense to me.
Comment #43
Gábor HojtsyAgreed, thanks for all the hard work on this!
Comment #44
webchickCommitted and pushed to 8.x. Thanks!
Comment #45
Gábor HojtsyThanks all!
Comment #46
maxtorete CreditAttribution: maxtorete commentedThanks all for reviewing and committing the patch :-)