Posted by mikewink on February 27, 2011 at 12:12pm
9 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | language system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | claudiu.cristea |
| Status: | needs review |
| Issue tags: | D8MI, language-content, Needs backport to 7.x |
Issue Summary
Right now the language select is placed "near the bottom" of the fields in the form. Reordering fields sometimes puts the select between other fields. Making the language select a pseudo field, implementing hook_field_extra_fields corrects this problem. The language can now be placed just like any other field, making it possible to place it into fieldsets (e.g. vertical tabs) too.
Attached is a simple patch that ads locale_field_extra_fields() support for the language select. I'm not quite sure if the condition to check when to add the language select to the extras array is sufficient. I took the same condition as the default locale_form_alter() uses.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| language_select_as_pseudo_field.patch | 866 bytes | Idle | PASSED: [[SimpleTest]]: [MySQL] 31,577 pass(es). | View details | Re-test |
Comments
#1
This sounds more like a feature request to me, although in D6 CCK provided the language pseudo field. So this could be considered an oversight while porting CCK in core. Let's hear what core maintainers have to say.
However:
+++ modules/locale/locale.module (revision )@@ -415,6 +415,29 @@
+ if (isset($type->type) && locale_multilingual_node_type($type->type)) {
The test should check if multilingual support is different from disabled otherwise we wouldn't have the language pseudo field for tanslatable nodes.
Powered by Dreditor.
#2
#3
#4
@mikewink:
The current patch cannot be committed as is. Why did you ignore #1?
#5
Hi plach,
I'm a bit confused. Tell me what to do and I will work on the patch? I don't know what's the problem in #1? There is a check in the patch and the patch passed the tests.
#6
The problem is that the current patch checks if multilingual support for the node type is "Enabled", but this excludes all node types having multilingual support set to "Enabled, with translation" (and ony other contrib additional mode). If the line cited in #1 is changed to check if multilingual support is not disabled the language pseudo field will appear everywhere it needs to.
#7
Bugs are fixed in the development version first, backported then.
#8
I actually think this should go into D7, otherwise every module dealing with content language (e.g. entity_translation and i18n_node) will have to provide its implementation.
#9
Because of future #375397: Make Node module optional we need to fix this in the Node module rather than Locale module.
#10
Here's a patch against Node module.
#11
Typo fix in title.
#12
Agreed. We also should consider #540294: Move node language settings from Locale to Node module. Cannot test this now, however:
+++ b/core/modules/node/node.module@@ -590,17 +590,24 @@ function node_add_body_field($type, $label = 'Body') {
+ // Add also the language select if Locale module is enabled
+ // and the bundle has multiligual support.
The comment should wrap at column 80.
+++ b/core/modules/node/node.module@@ -590,17 +590,24 @@ function node_add_body_field($type, $label = 'Body') {
+ 'description' => t('Locale module element'),
Shouldn't this be a "Node module element" too now?
24 days to next Drupal core point release.
#13
OK. Fixed your inputs from #12 , but postponing till #540294: Move node language settings from Locale to Node module. After #540294: Move node language settings from Locale to Node module is fixed, I will reroll the patch.
#14
Forgot the patch :)
#15
Looks like a great suggestion for improvement.
#16
Tagging for the content handling leg of D8MI.
#17
#540294: Move node language settings from Locale to Node module landed.
#18
The name of the node-type language variable has changed.
#19
Here's a patch to review.
#20
+++ b/core/modules/node/node.moduleundefined@@ -603,17 +603,25 @@ function node_add_body_field($type, $label = 'Body') {
+ $locale_enabled = module_exists('locale');
...
+ // Add also the "language" select if Locale module is enabled and the bundle
+ // has multilingual support.
+ if ($locale_enabled && variable_get('node_type_language_' . $bundle->type, 0)) {
Node language is not dependent on locale in Drupal 8 anymore, it merely needs language.module.
Also, what is this giving us exactly? Placement in the form and display? Can add language to display? Will it be added automatically to the render output after this patch?
#21
The title says all about this feature: Allow language select to be rearranged inside node form. It's only about the form. Many site builders need flexibility on placing the Language select. That's all.
See the screenshot.
#22
Looks good to me both on patch review and visually.
#23
We just got under thresholds tonight, so features are back on the table.
This patch seems reasonable to me. This is technically a data structure change in D7, but one could argue it's also a bug that it doesn't do this already.
Committed and pushed to 8.x and 7.x. Thanks!
#24
@webchick,
This patch cannot be applied to 7.x as it is... because:
I need to backport the patch first. Please revert the patch in 7.x because it will not work.
#25
Moving to 7.x too
#26
Whoopsie. :D
Reverted 7.x commit. Thanks for catching that!
#27
#28
@claudiu.cristea: the OP (first patch) looks like it would be a good start on a D7 backport?
#29
@Gábor Hojtsy,
Yes, I will fix that in Locale module for D7 in order to keep the logic before #540294: Move node language settings from Locale to Node module. That patch is a good stating point. I will come with a patch later today.
Thanks.
#30
Not on the D8MI sprint anymore.
#31
Rerolled.