- 🇺🇸United States jhodgdon Spokane, WA, USA
Note on priority: I am OK with demoting this to major if we are OK with admin/node really being broken for multilingual. But this issue is not that hard to fix and I don't think it's OK to release D8 with admin/node being broken for multilingual. It is definitely very broken -- the UI will lead people to delete entire nodes when they really only meant to delete a translation.
- 🇩🇪Germany dawehner
My thoughts about this are:
- Use the current content language of the side, if available for the row, otherwise fallback to the default langcode
- Don't show an exposed filter or any additional configuration
- At the same time, make a new issue, see #2474013: Make a node translation view → to show translations
Problem of the configuration
One problem which we might have here is that a lot of the functionality we would need is coupled to the language module, which is not enabled by default,
so adding a filter could cause problems. On the other hand, I don't see a reason that it might not be possible to add a language filter inside views itself.d) Maybe for multilingual we need a second view that shows node translations rather than individual nodes, and allows bulk/row ops appropriate to translations? In this view you could definitely filter by language, and you'd want the view to show each node in the row language. The ops would be something like "edit this translation" and "delete this translation".
Let's do that bit separately, I think that would be okay ...
- 🇫🇮Finland lauriii Finland
I tested whether its less confusing to have what @dawehner suggested and its way better than what we have at the moment
I attached a patch so people can try this out.
- 🇳🇱Netherlands stefan.r
Just a few ideas on this as I dealt with a similar issue when asked to put a language filter on a D7 Administration Views overview (and perhaps these would need to be addressed in the second "multilingual" view if we remove the language filter from the current overview altogether):
- Solutions such as an "only show original" filter sound targeted to site builders and may be confusing to content managers, who often won't care whether something is a translation or an original and may want the filtering to fetch both.
- If the real concern is that deleting a source node kills off all the translations, maybe we could retain that behavior in code but work around this in the UI somehow. For instance with separate "delete" and "delete with all translations" options, where a regular delete would only delete the current translation. I don't know that we have a way to deal with orphaned translations yet, but it seems like a useful feature to be able to delete an original translation. This would also solve the UI WTF in the node edit form of the original where if you click delete it says "Delete (this translation)" but actually deletes all of the translations, and the delete button being hidden on the translation overview page for a node.
- Even with the deletion problem solved, the table length would still be an issue, but seeing all the translations would often be rather "just enough information" than "too much information" for a site with only 3-4 languages. It also only applies to the unfiltered view. There ought to be no need to hide translations from the _filtered_ view. Further, if we're still going to only show nodes in the _unfiltered_ view anymore just to make the table less cluttered, maybe we can still find a way to make the translations selectable, for instance by making the rows collapsible, with the nodes expanding to all of their translations.
The last submitted patch, 3: node_admin_page_is-2473989-2.patch, failed testing.
- 🇩🇪Germany dawehner
-
+++ b/core/modules/node/config/optional/views.view.content.yml @@ -23,6 +23,12 @@ display: + options: + disable_sql_rewrite: false + distinct: true + replica: false + query_comment: '' + query_tags: { }
Yeah, let's not use distinct but rather filter the query properly.
-
+++ b/core/modules/node/config/optional/views.view.content.yml @@ -487,47 +496,6 @@ display: - langcode: - id: langcode - table: node_field_data - field: langcode - relationship: none - group_type: group - admin_label: '' - operator: in - value: { } - group: 1 - exposed: true - expose: - operator_id: langcode_op - label: Language - description: '' - use_operator: false - operator: langcode_op - identifier: langcode - required: false - remember: false - multiple: false - remember_roles: - authenticated: authenticated - anonymous: '0' - administrator: '0' - reduce: false - is_grouped: false - group_info: - label: '' - description: '' - identifier: '' - optional: true - widget: select - multiple: false - remember: false - default_group: All - default_group_multiple: { } - group_items: { } - plugin_id: language - entity_type: node - entity_field: langcode
So we would that move that querying into the new translation view ... @jhodgdon @gabor Are you okay with that?
-
- 🇫🇮Finland lauriii Finland
I didn't fix dawehners comments. Just tried to fix some tests.
The last submitted patch, 10: node_admin_page_is-2473989-10.patch, failed testing.
- 🇩🇪Germany dawehner
Thank you @lauriii for working on the issue!
I hope that working on those failures will still be worth, if we fix the problem in the right way.
- 🇨🇦Canada webchick Vancouver 🇨🇦
Screenshots would be helpful here to understand the issue. The issue summary sounds quite vast so it'd be nice if we could split out "must fix this to ship D8" (here) versus "other usability improvements" into separate issues.
- 🇩🇪Germany dawehner
Let's make the issue summary a bit better to understand.
- 🇬🇧United Kingdom catch
See #2428795-39: Translatable entity 'changed' timestamps are not working at all → and down. This needs feedback from Gabor/plach.
I'd expect this to work the 7.x entity_translation way too (one row per entity - then you can get to translations from the entity page itself), but I think it's 'by design' to show every translation here.
- 🇳🇱Netherlands stefan.r
@aspilicious by "an extra row with all the language codes" you mean "a row for every translation of a node" or do you mean "one row with all the translations in them"? I think the different coloring could work.
What are your thoughts on addressing the translations issue in the main "Content" view vs. removing all the language-related stuff from the main "Content" view and adding another "Content Translation" tab next to it?
I also talked to @dawehner about somehow addressing the source translation deletion issue. The problem is when we delete a source translation all its translations are deleted, if they were to remain intact though the problem is what to do with these "orphans" that don't have an "original language" anymore. Just to illustrate, if the original node is in dutch and the translations are in french/english, we may want to be able to delete the dutch translation while still preserving the french/english ones.
Right now deleting a source translation is disallowed in certain spots of the UI but allowed elsewhere. One solution would be to disallow it everywhere (including in bulk operations, where we can warn about this in the confirmation screen).
- 🇧🇪Belgium aspilicious
in Drupal 7 this is usually solved with one row for each entity and sometimes an extra column with all the language codes and the translated ones look different thas the others.
The current state is too confusing for most clients.
- 🇧🇪Belgium aspilicious
So something like this, strike through can be colors
My node title | article | published | fr es
- 🇳🇱Netherlands stefan.r
Yes that could work as far as the main overview is concerned. We may still want a separate translation overview in that case.
- 🇨🇦Canada webchick Vancouver 🇨🇦
The core committers discussed this today on a triage call.
Revisiting the definition of critical, https://www.drupal.org/node/45111 → , an issue is critical if it:
1. Renders [the] system unusable and have no workaround.
2. Causes loss of data.
3. Exposes security vulnerabilities.Obviously, 1) and 3) do not apply. 2) is interesting though, given this:
For instance, if you selected the French translation of node/1 and clicked Delete either in the row ops or with bulk ops, it would delete all translations, while the UI expectation would definitely be (since you could also see the Spanish and English versions there) that you're deleting just the French one.
We agreed that this is a bad enough usability problem that will result in unexpected loss of data, so a) from the issue summary is critical, so +1 to blocking D8's release on it. However, b-d sound like usability issues/improvements that are "normal" or at best "major" and we would never hold up the release of Drupal 8 on them.
So let's either repurpose this as a meta and split off sub-issues for a-d, or let's re-scope this to just a) and spin off sub-issues for b-d.
- 🇮🇹Italy plach Venezia
It would be nice to have Gabor's confirmation, but I think the plan so far has been to replicate as much as possible the node translation approach available in D7. The default core behavior for this screen in D7 is showing all translations, since they are single nodes. There is a language filter that allows to filter on the node language, which means one can show only French nodes. Bulk operations and operation links refer to nodes, that is to individual translations.
As we saw once more in the comments above, when dealing with multilingual sites there is no one-size-fits-all solution. Depending on the use case, one might want to display all translations, just the original ones, or only the ones matching the current content (or UI) language with/without fallback to the original ones, just to name the most common scenarios.
Since we are not able to replicate the D7 content admin page atm, I think it would be sensible to try to achieve that. This would have two advantages:
- it would make the default D8 UX similar to the D7 one many users are already familiar with;
- it would provide the missing pieces to be able to support all the use cases presented above.
As already outlined above, to replicate the D7 behavior we mainly need to address two issues:
- Fix operation links to deal with translations. For regular links like view or edit, we just need to use the translation language to trigger a language switch, this would make the user reach the related page in the expected language, while on monolingual sites this would have no effect. The deletion link is a bit trickier: I think the best solution would be make Content Translation swap the plugin class and replace it with a specialized one able to distinguish between original and translation: in the former case the link route would stay the same, in the latter it would change the route of CT's translation deletion page. I agree with @stefan.r that we should probably aim to handle all translations the same way, but I guess this could be a non-critical follow-up, since CT could just inject a message on the entity deletion form warning the user that all translations will be deleted. Otherwise we'd need to implement something similar to what we have in D7 and elect a new original when the original translation is deleted, which might not be completely trivial.
- Fix bulk operations to deal with translations. I think in this case only deletion needs special care, as the other operations should already behave correctly if the fields they apply to are configured as translatable. If they aren't they apply to all translations, thus again we should be fine. In this case we could simply remove translations or the whole node depending on the user selection.
Btw, I'm not sure what's so confusing about the language filter: assuming it applies to the translation language, it looks pretty intuitive to me.
- 🇳🇱Netherlands stefan.r
Yes that makes sense, I think D7 Adminstration Views retains this same behavior right?
In terms of this issue just a warning message on the confirmation screen would suffice I guess. If we want the "elect a new original" functionality, instead of a warning it could be a dropdown for every single source translation on that same confirmation screen, with some sensible defaults (either user defined or using already existing language weightings)
- 🇩🇪Germany dawehner
Right, let's create a first subissue: #2476563: Entity operations links tied to original entity language, ignore everything else →
- 🇩🇪Germany cpj
Just to agree with what @plach is saying in #22
The default core behavior for this screen in D7 is showing all translations, since they are single nodes. There is a language filter that allows to filter on the node language, which means one can show only French nodes.
We have been running a 2 language D8 site in production for over a year (since Alpha-6, now Beta-7) and we have the admin view setup as described here, with a language filter to toggle between all nodes or just nodes in a particular language. The other proposed improvements would definitely make life easier too.
- 🇩🇪Germany cpj
And here's a screen shot from the admin content view of the D8 site, showing the additional language filter
- 🇩🇪Germany dawehner
@cpj
Do you think we should default this exposed filter to filter by the original language? - 🇩🇪Germany dawehner
@cpj
Do you think we should default this exposed filter to filter by the original language? - 🇩🇪Germany cpj
@dawehner - I'm not sure what's the best starting default, but a way to make the language filter selection sticky for the session would be useful (not essential, just helpful). For our clients who have multilingual sites (usually dual language) they are used to seeing both sets of nodes as the starting default and filtering from there.
- 🇳🇱Netherlands stefan.r
The best defaults will likely depend on the use case again, if we want to stay close to D7 we could filter by "the displayed nodes have either a translation or an original version in the selected language", and not apply the filter by default (ie showing all languages).
- 🇩🇪Germany dawehner
@amateescu and myself talked about that during break and we thought about the following:
- Let views continue to show all translations, because translations are a first class citizen in Drupal now
- Given that we have to fix the entity operations links and bulk operations, well that was already the case anywhere
- We should provide the exposed filter which allows you to filter by current source and specific language
We'll try to build that in order to move forward.
@cpj
Its amazing, and kinda suprises me, that views already have the store the value per session feature :) I totally forgot about that - 🇷🇴Romania amateescu
This patch does two things:
- adds a language field to the view
- makes both the language field and filter hidden by default if the site is not multilingual (i.e. Drupal\Core\Language\LanguageManager::isMultilingual() === FALSE)@dawehner is working on adding test coverage, so leaving at NW for that.
The last submitted patch, 34: 2473989-test.patch, failed testing.
The last submitted patch, 34: 2473989-34.patch, failed testing.
- 🇩🇪Germany cpj
@dawehner #32 - I'm not actually sure if views in D8 do currently have the store-the-value-per-session feature. I was suggesting it as a nice-to-have here.
- 🇩🇪Germany dawehner
It does, see
\Drupal\views\Plugin\views\filter\FilterPluginBase::storeGroupInput
- 🇮🇹Italy plach Venezia
I added some screenshots to the IS, to summarize the current UI changes. Basically we hide the language filter on monolingual sites and display a language column on multilingual sites. These are nice and welcome UX improvements that bring us closer to the D7 UX, but definitely do not solve the critical side of this issue (data loss since one might thing she's deleting a translation and the whole entity is deleted instead), which are addressed in #2476563: Entity operations links tied to original entity language, ignore everything else → .
I think we should demote this to major and promote the other to critical or merge them back into this one. If we keep them separate, which makes sense too, this should be almost RTBC.
Quick code review:
-
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php @@ -122,7 +122,12 @@ protected function viewValue(FieldItemInterface $item) { + // Languages like L + // Values like \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_SPECIFIED + // and \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_APPLICABLE are + // not returned from the language manager above. + $name = isset($languages[$item->language->getId()]) ? $languages[$item->language->getId()]->getName() : $item->language->getId();
This comment is messed up and does not wrap at column 80. Would it make sense to avoid fully-qualified class names?
-
+++ b/core/modules/views/src/Plugin/views/field/FieldLanguage.php @@ -0,0 +1,34 @@ + // Don't display the field in case the site is not multilingual, because + // there is no point in doing so. +++ b/core/modules/views/src/Plugin/views/filter/LanguageFilter.php @@ -72,13 +73,14 @@ public function getValueOptions() { + // Don't display the filter in case the site is not multilingual, because // there is no point in doing so. --- a/core/modules/views/src/Tests/Handler/FieldFieldAccessTestBase.php +++ b/core/modules/views/src/Tests/Handler/FieldFieldAccessTestBase.php
Can we have a slightly more meaningful comment? Something like "No point in displaying the language field/filter on monolingual sites, as only one language value is available."
-
- 🇨🇦Canada yang_yi_cn
The screen shots of the patched version makes senses. I manage a lot of multilingual sites in Drupal 7 and I always creates a view similar to the patched version to replace the default node admin page.
It does make more sense to have "language" before the "title" search box because you want to ensure the language is right before your type the keyword to search.
On my views I have the "language" filter right after the "publish" filter, even before "type", but I guess after "type" is ok too. In my logic maybe when I try to find a content I always think language first, then the type, e.g. I will want to find something in English, in News or Blog types, with some keywords in title.
- 🇭🇺Hungary Gábor Hojtsy Hungary
I quickly skimmed the comments here. I don't necessarily agree with some of the assessments of the summary. Whether this view should show nodes or translations is use case dependent. @stefan.r wrote in #4:
Solutions such as an "only show original" filter sound targeted to site builders and may be confusing to content managers, who often won't care whether something is a translation or an original and may want the filtering to fetch both.
Drupal 7 showed translations as individual elements here as well. Also @dawehner pointed out some technical problems with optional views elements. We used to have language elements in this view before but those were dependent on language module. I think we moved views language elements to the views module since then, so it may be possible to add a column back without needing to depend on language module (or fail at schema validation).
I don't necessarily agree that the language filter is useless on "monolingual" sites, it depends on how you define monolingual. The language system supports two special languages additionally to the one configured even on monolingual sites, so you may already have nodes in one of three languages without needing to configure one more. Again this depends on the use case.
I do agree that we need to fix that operations and bulk operations need to be translation aware. Ideally we should be able to make views where operations and bulk operations act on the whole entity or act on the translation appropriately. Not sure how best to do that, but that sounds like the territory of #2476563: Entity operations links tied to original entity language, ignore everything else → now. Should this be marked postponed on subissues then?
- 🇩🇪Germany dawehner
Well yeah, the current patch improves some of the usability, but it certainly doesn't fix the critical side of this issue.
- 🇩🇪Germany dawehner
Added a subissue for the bulk operations side of the problem space: #2484037: Make Views bulk operations entity translation aware →
- 🇮🇹Italy plach Venezia
Given the other sub-issues I think this is no longer critical.
- 🇳🇱Netherlands stefan.r
Some D7 screenshots, for reference:
@plach is this what we're trying to replicate? Or are we thinking of Entity Translation?
- 🇨🇦Canada webchick Vancouver 🇨🇦
Core committers spoke today about this and related issues.
Given that "the simplest thing that can possibly work" to fix the admin/content page is to switch it so it's showing nodes instead of translations—then all the underlying assumptions work fine, and the form works the same regardless of multilingual sites or not—spun off #2485159: Switch admin/content from showing one translation per row to one node per row → as a new critical issue, essentially for a) in the list of proposed resolutions here.
Re-purposing this one to talking about "Views that provide lists of translations" since all of this is still an issue for those, but after the above issue core will not ship with one, so not something we have to resolve before release. Will downgrade all the children of this one as a result. (They're still great things to fix, mind you.)
- 🇳🇱Netherlands stefan.r
Even if we just list nodes in the standard content view, this still causes data loss in custom views.
So if just before release time we still don't have a fix, we should probably revisit this issue and just hide operation links/bulk operations from any view that includes translations.
- 🇮🇹Italy plach Venezia
Core committers spoke today about this and related issues.
It would have been preferable to involve the initiative lead or at least the subsystem maintainer (me) in this discussion, before making such an important decision. As stated in #2485159-4: Switch admin/content from showing one translation per row to one node per row → , I don't think the solution proposed there is valid nor viable, because after it's done we are just one click away from recreating the very same UX issue we are trying to fix with #2484037: Make Views bulk operations entity translation aware → and #2476563: Entity operations links tied to original entity language, ignore everything else → .
- 🇩🇪Germany cpj
#51
"the simplest thing that can possibly work" to fix the admin/content page is to switch it so it's showing nodes instead of translations—then all the underlying assumptions work fine
I also don't think this is correct, for similar reasons to those mentioned by platch in #54:
I thought we were close to a consensus on the solution path here, so not sure what's gained by moving the discussion to #2485159-4
- 🇩🇪Germany dawehner
Quick reroll, as it might be still useful in the future.
- 🇭🇺Hungary Gábor Hojtsy Hungary
I think people reviewing the other issues were concerned the language was not visible in the table when doing operations it was not clear which variant they are doing the operation to (especially if the title is not translated for some reason, eg. a person's name).
- 🇺🇸United States jhodgdon Spokane, WA, USA
Yes, and in the case of User accounts (the admin/people page has the same problem if you make User entity translatable), it is *really* not obvious the difference between the translations, because all you see is the username, which is untranslatable.
- 🇭🇺Hungary Gábor Hojtsy Hungary
@jhodgdon: following #2484037: Make Views bulk operations entity translation aware → (now in head) the user view does not have that problem.
@dawehner/@plach:
-
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php @@ -122,7 +122,12 @@ protected function viewValue(FieldItemInterface $item) { - return $item->language ? SafeMarkup::checkPlain($languages[$item->language->getId()]->getName()) : ''; + // Languages like L + // Values like \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_SPECIFIED + // and \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_APPLICABLE are + // not returned from the language manager above. + $name = isset($languages[$item->language->getId()]) ? $languages[$item->language->getId()]->getName() : $item->language->getId(); + return $item->language ? SafeMarkup::checkPlain($name) : '';
The language list should include non-configurable ones too, if the right argument is passed to getLanguages() et. al. LanguageInterface::STATE_ALL is the right flag.
-
+++ b/core/modules/views/src/Plugin/views/field/FieldLanguage.php @@ -0,0 +1,34 @@ + public function access(AccountInterface $account) { + // Don't display the field in case the site is not multilingual, because + // there is no point in doing so. + if (!$this->languageManager->isMultilingual()) { + return FALSE; + } +++ b/core/modules/views/src/Plugin/views/filter/LanguageFilter.php @@ -74,13 +75,14 @@ public function getValueOptions() { - public function query() { - // Don't filter by language in case the site is not multilingual, because + public function access(AccountInterface $account) { + // Don't display the filter in case the site is not multilingual, because // there is no point in doing so. if (!$this->languageManager->isMultilingual()) { - return; + return FALSE; }
While these are great for an 80% case (and it would really be nice to fix this issue with them), we should somehow consider the less common case that even on a "non-multilingual site" where you only have 1 configured language, there are 2 non-configurable languages, to there still may be entities in 3 different languages. While that is not very common, it is certainly a possibility and this way people have no way to expose the language field / filter without hacking core. That sounds like a problem :/
So once again this looks great for the 80% (probably more) use case, but if we can resolve this in way that would work for everyone, that would be great.
-
- 🇭🇺Hungary Gábor Hojtsy Hungary
Retitled for the scope it is attempting to fix. The actual operation links are also a problem, I have a fix going in #2504663: Entity operations links tied to original entity language, ignore both views row and UI languages → . Reviews needed :)
- 🇳🇴Norway matsbla
I tested patch in #57 and it worked good! Here are some comments/ideas:
- Maybe it should be possible to filter by "Source Language". It could be good to be able to see "all translations that are source languages", but also to see "all nodes that have language X as source language". In TGMGT module they have one filter field for "source language" and another for "target language". It might seem like something only needed in special cases, but I think when doing multilingual websites it can be important to have possibility for a very fine adjusted translation filter.
- Now we can only filter by published/unpublished, but would be great to be able to filter on other types of status like outdated translations/up-to-date translations, promoted/not promoted, sticky/not sticky. It is possible in D7. It would be best if it is possible to combine these filters. Lets say you have several translations marked as outdated, maybe you want to find all translations that are "published + outdated translation", or even "published + outdated translation + promoted".
- Maybe also consider to make it possible to filter on revision status? Not sure if this would be the same as draft/unpublished. To me it seem like a node can have both a published version ('Set current') and a draft version:
#2429153: On node revision overview, use 'Set current' if revisions are newer than current version →
#1776796: Provide a better UX for creating, editing & managing draft revisions. →- Why only make it possible to search for keywords in title? Would be better to make it possible to search for keywords in the whole nodes, but that's probably another issue.
- 🇭🇺Hungary Gábor Hojtsy Hungary
@matsbla:
Thanks for testing! This is a general content management view, so making translation specific changes is not necessarily nice, the language stuff is added to make it easier to understand, not to make it a full fledged translation view. That would be its own view I think or if someone desires this one to be it, they can modify it manually. For manual modifications, most of what you explained should already be possible. W have #2450195: Original language of entities not accessible in views anymore → open for the source language not being possible at the moment. We used to have that option earlier and lost it on the way.
- 🇳🇴Norway matsbla
But why are status alternatives like "published", "promoted" and "sticky" not part of the "status" field? I guess they are not "translation specific"? Or it could at least be handy alternatives also for a monolingual website?
The filter field named "Published status" in D8 have the much more generic label "Status" in D7. I think it is better and more flexible, and bringing back the alternatives from D7 would make the UI much easier for those used with D7.
D7 also let you combine different types of status in the search:
- 🇭🇺Hungary Gábor Hojtsy Hungary
@matsbla: you requested filters for "outdated translations/up-to-date translation" which is what I referred to. The other filters may be nice indeed, they are not related to making this views better for language support, so not in scope on this issue.
- 🇭🇺Hungary Gábor Hojtsy Hungary
Removing from sprint due to unfortunate lack of interest.
- 🇨🇭Switzerland dasjo Zurich
I just had another look into this topic. Not sure if this is the right place but what I'm missing is the option to have a view that would display contents in the current language or its source language, if the content hasn't been translation yet to the current language.
- 🇭🇺Hungary Gábor Hojtsy Hungary
@dasjo: that could be a view row display setting solution, since you are not actually filtering for specific languages, you just want unique results per entity ID and then decide which language to use to render it. If you look at TranslationLanguageRenderer::preRender() that is where the rendering is passed over to the entity manager provided view builder. Unfortunately it does not qualify that its being rendered as a views row, even though then you could implement a specific fallback method for this scenario. Anyway, I encourage you to look around the views row renderers for your use case.
- 🇺🇸United States jhodgdon Spokane, WA, USA
I just tested this on the latest -dev code.
All of the problems in the issue summary have been resolved due to fixes from other issues, at least to my satisfaction.
The Content page does still show all of the translations of all of the content, but it works MUCH better:
- The language filter filters you down to just the translations in the language you choose.
- The operations are at the translation level -- for instance, if you choose delete on a translation, you are just deleting that one translation. If you choose delete on one of the the original language rows, you get a coherent warning telling you you're about to delete the [specific languages listed] translations of that node.So to me, this is working in a way that makes sense. I think we can call this issue fixed.
- 🇮🇹Italy plach Venezia
@jhodgdon:
It would still be nice to add a dynamic language column to the node content view, but that's definitely 8.1 material. Would it make sense to repurpose this issue, since we already have a lot of background and followers here?
- 🇭🇺Hungary Gábor Hojtsy Hungary
@jhodgdon: well the premise of this issue was that the language filter does not make sense on monolingual sites and the lack of language column is confusing on multilingual sites, so in some ways the current middle road is not ideal for either. Its not bad per say, but its not ideal. See screenshots in summary. I think if this is to be closed, it would be won't fix, its definitely not fixed.
- 🇺🇸United States jhodgdon Spokane, WA, USA
Ah, OK. Well it needs an issue summary update, because much of what is in the summary has been taken care of.
- 🇨🇴Colombia jomarocas
Hi, i have a similar issue with you, i have two languages, and i create content but later i see error with translate, if it not translated well, or is a core drupal error, i attache images with this error
the problem is with date create and updated, thanks if this issue works my issue
- 🇺🇸United States xjm
The core committers and Views maintainers (alexpott, xjm, effulgentsia, tim.plunkett, and dawehner) agreed that this is a major issue. Similarly to 🐛 Duplicated rows on admin/content if author is translated Active , it's a prominent and confusing bug on one of core's most important administration forms for multilingual sites.
- 🇨🇭Switzerland berdir Switzerland
i think @bojanz wrote his own solution for this problem in Commerce 2.x, with filters and fields that are only displayed if relevant (e.g. don't show a type field/filter if there's only one type).
I assume he simply solved that by having access logic for those plugins that hides them if not applicable. See https://drupalcommerce.org/blog/43978/commerce-20-alpha2-released.
Seems like a fairly simple thing that we could also use for those plugins if we move them out of the optional modules and make them always available?
- 🇨🇭Switzerland berdir Switzerland
@dawehner actually pointed out to me that the patch in #57 is doing pretty much that. Needs a reroll, I think the language formatter fix was already fixed elsewhere.
- 🇭🇺Hungary Gábor Hojtsy Hungary
Agreed that its needs a reroll, indeed it does conditional logic like you explained :)
The last submitted patch, 79: lack_of_dynamic-2473989-79.patch, failed testing.
- 🇩🇪Germany dawehner
In general I still kinda like the solution to add those fields always, but just let them do nothing, in case we are not on a multilingual site.
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php @@ -122,9 +122,12 @@ protected function viewValue(FieldItemInterface $item) { + // Languages like L
This is a bit truncated ;)
- 🇧🇪Belgium swentel
The fails errored because the SafeMarkup class was not found. Went for different approach though (because then I could get the tests working, at least one)
The last submitted patch, 82: lack_of_dynamic-2473989-82.patch, failed testing.
- 🇭🇺Hungary Gábor Hojtsy Hungary
@swentel: good trick, yay! any ideas for the remaining two fails? one says null constraint on node title :)
- 🇧🇪Belgium swentel
@Gábor Hojtsy Looked at this weekend, no clue :/
Can't reproduce it manually either. Drupal 8.0.6 → was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 → is now available and sites should prepare to update to 8.1.0.
Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
- 🇬🇧United Kingdom martin107
1)
Indirect modification of overloaded element of Drupal\Core\Field\FieldItemList
That is fixed by using
$node->setTItle()
instead of
$node->title[0]->value
NodeAdminTest now passes.
2) If we are swapping out "Engilsh title" we should replace it with "Spanish title"
The last submitted patch, 86: lack_of_dynamic-2473989-86.patch, failed testing.
The last submitted patch, 86: lack_of_dynamic-2473989-86.patch, failed testing.
The last submitted patch, 91: edit_issue_lack_of-2473989-91.patch, failed testing.
- 🇪🇸Spain dimaro Seville, Spain
I retested this issue.
Tagging as Needs reroll. - 🇮🇳India Sonal.Sangale
Rerolled the patch.
There were no conflicts. Hence interdiff is not added.
Drupal 8.1.9 → was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 → is now available and sites should prepare to upgrade to 8.2.0.
Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
- 🇩🇪Germany dawehner
This looks perfect for me.
-
+++ b/core/modules/node/config/optional/views.view.content.yml @@ -60,10 +60,8 @@ display: - edit_node: edit_node - delete_node: delete_node - dropbutton: dropbutton - timestamp: title + langcode: langcode + operations: operations info: node_bulk_form: align: '' @@ -105,28 +103,14 @@ display: @@ -105,28 +103,14 @@ display: separator: '' empty_column: false responsive: priority-low - edit_node: + langcode: sortable: false default_sort_order: asc align: '' separator: '' empty_column: false responsive: '' - delete_node: - sortable: false - default_sort_order: asc - align: '' - separator: '' - empty_column: false - responsive: '' - dropbutton: - sortable: false - default_sort_order: asc - align: '' - separator: '' - empty_column: false - responsive: '' - timestamp:
These changes are perfectly fine, as we have operations down below since a while.
-
+++ b/core/modules/views/src/Plugin/views/field/FieldLanguage.php @@ -0,0 +1,34 @@ + +/** + * @file + * Contains \Drupal\views\Plugin\views\field\FieldLanguage. + */
Nitpick: Can be fixed on commit ...
-
The last submitted patch, 33: 2473989-33.patch, failed testing.
Drupal 8.2.6 → was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. ( Drupal 8.3.0-alpha1 → is available for testing.)
Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
-
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php @@ -117,9 +117,11 @@ protected function viewValue(FieldItemInterface $item) { + $name = isset($languages[$item->language->getId()]) ? $languages[$item->language->getId()]->getName() : $item->language->getId(); + return $item->language ? ['#plain_text' => $name] : '';
How can $item->language not be truthy and
$item->language->getId()
not fail sometimes? -
+++ b/core/modules/views/src/EntityViewsData.php @@ -397,7 +397,7 @@ protected function mapSingleFieldViewsData($table, $field_name, $field_type, $co - $views_field['field']['id'] = 'field'; + $views_field['field']['id'] = 'field_language';
This requires a cache clear no? I guess we're only considering this for 8.3.x - in which case one already exists.
-
+++ b/core/modules/views/src/Plugin/views/field/FieldLanguage.php @@ -0,0 +1,34 @@ +/** + * @file + * Contains \Drupal\views\Plugin\views\field\FieldLanguage. + */ +
Not needed.
-
+++ b/core/modules/node/src/Tests/NodeAdminTest.php @@ -2,6 +2,8 @@ +use Drupal\Core\Language\Language;
Not used.
-
- 🇳🇴Norway vegardjo
Just a small note on the patch from #97, there is a mixup in the last 3 digits: It's named 2473898-97.patch while it should be named 2473989-97.patch.
- 🇪🇸Spain Peacog
I've addressed @alexpott's and @vegardjo's comments in this re-roll :)
- 🇪🇸Spain Peacog
Re-roll needed because NodeAdminTest.php directory was changed in 8.3.3.
Drupal 8.5.6 → was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. ( Drupal 8.6.0-rc1 → is available for testing.)
Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
Drupal 8.4.4 → was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. ( Drupal 8.5.0-alpha1 → is available for testing.)
Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
Drupal 8.3.6 → was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. ( Drupal 8.4.0-alpha1 → is available for testing.)
Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule → and the Allowed changes during the Drupal 8 release cycle → .
- 🇩🇪Germany szeidler Berlin
I can confirm, that the patch in #109 is working in 8.4.x as expected and addressed the issues in #105. Only
This requires a cache clear no? I guess we're only considering this for 8.3.x - in which case one already exists.
remains unanswered.
- 🇮🇳India msankhala Bikaner, Rajasthan
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php @@ -117,9 +117,17 @@ protected function viewValue(FieldItemInterface $item) { + // Values like \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_SPECIFIED + // and \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_APPLICABLE are + // not returned from the language manager above.
The line should not exceed 80 characters. This can be rephrased as:
// \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_SPECIFIED // and \Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_APPLICABLE are // not returned from the language manager above.
- 🇺🇸United States shaal Boca Raton, FL
Drupal 8.9.0-beta1 → was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule → and the Allowed changes during the Drupal 8 and 9 release cycles → .
Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule → and the Allowed changes during the Drupal 8 and 9 release cycles → .
- 🇳🇱Netherlands seanB Netherlands
Should we add the language column for the media and media_library views in this patch as well? Or should we have a followup for those?
Drupal 9.4.0-alpha1 → was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .
Drupal 9.3.0-rc1 → was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .
Drupal 9.2.0-alpha1 → will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .
Drupal 9.1.0-alpha1 → will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule → and the Allowed changes during the Drupal 9 release cycle → .
- 🇧🇪Belgium baikho Antwerp, BE
Added reroll of patch #119, fixed few tests, removed needs reroll tag.
Drupal 9.5.0-beta2 → and Drupal 10.0.0-beta2 → were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .
- 🇳🇿New Zealand quietone
That is tagged D8 upgrade path and D8 is EOL. So, changing the 'upgrade path' and not tie this to a specific version.
- 🇬🇧United Kingdom rivimey
7 years, 2 major Drupal versions.... sigh.
Having looked at the last review which set needs work and the latest patch, which addresses the only critical change mentioned, I feel this patch is overdue for Needs Review. Given various people have been rerolling it it seems the patch(es) are being used, and so quite possibly this should be RTBC. One step at a time though!
Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened → , as Drupal.org infrastructure cannot currently fully support a branch named
main
. New developments and disruptive changes should now be targeted for the11.x
branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- First commit to issue fork.
- 🇮🇳India srishtiiee
Updated the issue summary to remove all the stuff that has been taken care of in other issues.
Removed theUpgrade path
tag since the admin UI views are owned by the site, and we don't need to provide for it. If at all the upgrade paths for the field is required, it can be done in a follow-up. - Status changed to Needs review
11 months ago 4:00am 14 December 2023 - 🇺🇸United States smustgrave
UI views are owned by the site, and we don't need to provide for it
Not sure I follow we usually have to write upgrade paths for admin views before?
- 🇩🇪Germany rkoller Nürnberg, Germany
i've applied MR5708 to 11.x-dev with two languages (english and german) installed. before i'Ve applied the patch i've enabled the content translation for the article content type and created an article node for english and german. the following things i've noticed.
the language select field is shown for the filter component. but i am unable to see the language column anywhere?
I've then removed the german language. on the content page the language select field within the filter component got properly removed.
i've then readded german as a language. on the content page the language select field is shown again but still no sign of the language column. based on the screenshots provided in #135 🐛 Lack of dynamic language field / filter makes shipping core views hard to be both compatible with mono and multilingual Needs review it should be there as well?
and should the language names in parenthesis in the title as illustrated in https://www.drupal.org/files/issues/2023-12-14/MR_multilingual.png → also be shown? or was that manually added on node creation to easier visually validate that the language shown in the column is the correct one?
- 🇫🇮Finland lauriii Finland
#140: Did you re-install Drupal to test this? This is only available for new installations at the moment. #139 tagged this for a follow-up to discuss if we should provide a upgrade path for this. Generally we don't provide upgrade paths for the admin view changes but this seems like a significant enough UX improvement that this might warrant one.
- 🇬🇧United Kingdom catch
Agreed with a follow-up issue for the upgrade path. Ideally we should add it, but the worst case if we don't is things are better for new sites, not worse for existing sites, and existing sites can manually make the change.
- 🇨🇭Switzerland berdir Switzerland
> Not sure I follow we usually have to write upgrade paths for admin views before?
Without having looked at what the patch is doing exactly. We do technical updates for new settings for a given filter or field plugin, to make sure it matches the default config. We do not add new elements to a specific view because we have no idea if that view even exists, in what form it has been customized and what not. That's what meant with views are owned by the site.
- 🇩🇪Germany rkoller Nürnberg, Germany
it was a fresh install of drupal but the patch got applied after
i've applied MR5708 to 11.x-dev with two languages (english and german) installed. before i'Ve applied the patch i've enabled the content translation for the article content type and created an article node for english and german.
so you mean that the patch would have to be applied right before even the site gets installed? i can test that as well.
but on the other hand in that case a +1 for an upgrade path to the admin view. otherwise that functionality wouldnt be available for any existing site and it looks like a useful one?
- 🇫🇮Finland lauriii Finland
I agree that having an upgrade path would be useful but there's going to be some complexity with that. Drupal has decided to take the trade-off that we make it easy for site builders to customize the admin UI with the down side that it's significantly harder for us to make improvements to the experience. We could potentially check if the view was unmodified and apply the upgrade path in that case but that's for the upgrade path to define.
- 🇩🇪Germany rkoller Nürnberg, Germany
I agree. But based on the comments in #142 and #143 as well as the explanations that @catch provided in a thread on the #needs-review-queue-initiative channel i understand the problem better why an upgrade path usually is not provided. but still a +1 if it would somehow be viable and possible in a followup with for example the approach you've outlined to provide an upgrade path . cuz from a users/site builders perspective i see the danger that this change might go completely unnoticed otherwise.
and i've installed a complete new site now and applied the patch before i've installed the site. that way the select field as well as the language column are shown. i've removed the second language, select field and column were removed and when i re added a second language select field and column were shown again. from a manual testing perspective it looks great.
- Status changed to Needs work
11 months ago 7:36am 15 December 2023 - Status changed to Needs review
11 months ago 9:15am 15 December 2023 - 🇮🇳India srishtiiee
Created a follow-up for the upgrade path: 📌 Add an upgrade path for the language field in admin content view Active
- Status changed to RTBC
11 months ago 10:07am 15 December 2023 - Status changed to Needs work
11 months ago 3:10am 30 December 2023 - 🇳🇿New Zealand quietone
I'm triaging RTBC issues → . I read the IS and the comments and the MR.
I found that #40.2 was not addressed. I unresolved one comment in the MR that where I didn't find a response. Perhaps I missed it. And there are other comments in the MR from my review.
Setting to Needs work.
- 🇳🇿New Zealand quietone
And it looks like there is need of a followup for #41/#42 to move the language filter to before title. from @plach
Thanks to everyone who has worked on this 9 year old issue (I do enjoy working on the old ones). Especially to @dawehner, @plach, @stefan.r and @srishtiiee, who updated the Issue Summary. That makes an enormous difference to reviewers and committers. An up to date summary is a real time saver!
- Status changed to Needs review
10 months ago 5:59pm 4 January 2024 - 🇮🇳India narendraR Jaipur, India
Addressed all feedback and follow-up created 📌 Move the language filter before title on 'admin/content' page Active
- Status changed to Needs work
10 months ago 8:02pm 7 January 2024 - Status changed to Needs review
10 months ago 6:15am 8 January 2024 - 🇮🇳India kunal.sachdev
kunal.sachdev → made their first commit to this issue’s fork.
- Status changed to RTBC
10 months ago 1:01pm 19 January 2024 - 🇮🇳India kunal.sachdev
Applied one suggestion and overall the changes looks good to me.
- Status changed to Fixed
9 months ago 1:33pm 29 February 2024 - 🇬🇧United Kingdom catch
This looks good to me. Agreed with not adding an upgrade path - we don't know what customizations have already been made by sites, and a site wanting the improvement can just edit a view.
Couldn't decide whether to backport this to 10.2.x, given it changes views data a bit, decided to leave it in 10.3.x.
Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!
Tried my best with commit credit but since this is a very long issue with a lot of contributors, I may have overlooked someone and apologies if so.
Automatically closed - issue fixed for 2 weeks with no activity.