Problem/Motivation

When on a translated page, hit edit for a block. You'll get to edit the original text, not the translated text. The same applies also to any other kinds of configuration override, whether from settings.php or domain based or group based or time of day based or whatever. Not sure how could settings tray conceptually be ready for this, given the config system itself does not know either which parts of which config belong to which override source. It is clear though that for multilingual sites, the settings tray does not deliver on its promise and does not let you edit what you see (neither does it give you a way to find out where to edit that).

Screenshot from a site installed in English with https://simplytest.me/project/multilingual_demo/8.x-2.0

Proposed resolution

Don't know.

Remaining tasks

N/A

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy created an issue. See original summary.

Gábor Hojtsy’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

Title: Settings tray conceptually incompatible with configuration overrides » Settings tray conceptually incompatible with configuration overrides such as translations
Gábor Hojtsy’s picture

Component: other » outside_in.module
dawehner’s picture

Priority: Normal » Major

That sounds at least major for me.

Gábor Hojtsy’s picture

Issue summary: View changes

Version: 8.2.x-dev » 8.3.x-dev

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.

tedbow’s picture

Ok. Here is rough patch that tries to solve this for translations:
Here is what it does so far:

  1. Opens the quick edit form for the block as before
  2. alerts you if the current page is using a translations
  3. lists all the languages with links to edit, add, delete translation as needed
  4. Opens above links in the settings tray

Here a couple problems.

Some thoughts

  • Should we only link to the current translation language in the tray not list all languages.
  • Should we do nothing at all if you are looking at the original language of the block?

Screenshots

Wim Leers’s picture

Issue tags: +D8MI

Should we only link to the current translation language in the tray not list all languages.

I think you should make the current translation editable right away, otherwise the point of Outside In is mostly lost for all multilingual sites, which is a large fraction of Drupal sites…

But it may still be valuable to have some UI to link to the other translations. Just make the current translation immediately editable.


However…

Not sure how could settings tray conceptually be ready for this, given the config system itself does not know either which parts of which config belong to which override source.

I have an idea. To have config overrides, you need a \Drupal\Core\Config\ConfigFactoryOverrideInterface service that has a config.factory.override tag. Therefore we could solve this problem as follows:

  1. outside_in should add a container compiler pass that runs late.
  2. This compiler pass should check every config.factory.override-tagged service.
  3. It should create a list of all these services, and store that in a container parameter.
  4. The outside_in module can then check that that container parameter has only one entry: language.config_factory_override. If that's the case, we can make multilingual work just fine.
  5. However, if there are more, we need a way to signal that some of these are compatible with Outside In. The easy approach would be to add some sort of Outside In flag to those service definitions. But that requires all those modules to be specifically aware of Outside In, which is bad. So, alternatively, Outside In can detect compatibility automatically: it can invoke ::loadOverrides() on each of those config overrider services with the config data for the object that the user is trying to edit. If the empty array is returned, Outside In knows that this config override event subscriber is not overriding this, and hence it's safe for the Outside In module to offer its Outside In experience.
Gábor Hojtsy’s picture

Yeah, imagine you have organic groups (overrides per group for your block) and languages (overrides per language for your block). Linking to a different language may or may not have a translation but may still be different from the global config for the block due to the organic group override.

Wim Leers’s picture

Yep. But config translation is roughly used by >=50% of D8 sites. Organic Groups/Domain/… only on a single-digit percentage. Therefore, if Outside In could just support Config Translation, then it'd be usable on >=90% of D8 sites. So that makes this worthwhile IMO. I think the outline I sketched in #10 suggests it is feasible.

tedbow’s picture

Issue tags: +Usability
Wim Leers’s picture

I think Usability is kind of underselling it, no?

I'd think incompatibility with Config Translation makes it "Outside In-critical"?

tedbow’s picture

re @wim leers in #10

I think you should make the current translation editable right away, otherwise the point of Outside In is mostly lost for all multilingual sites, which is a large fraction of Drupal sites…

This patch is try at that.

Here is short video that shows the flow described below https://youtu.be/z3t-fBKdUaY

Here is block with current translation in effect.

We looking at example.com/es. There is spanish override for this block.
At the top of the form we embed the Config Translation for the language that is being used. This will only show if the block has override for the current language.
** this form won't actually work now but if we decide this is the way to go we can fix that

At the bottom is a closed fieldset "Other Translations".

When open it lists all other translation except the current override.

Clicking any of the links here will open up the form in the tray.

If we look at the same block but now but in the default language, English

We just see "Translation" fieldset collapsed at the bottom

Opening up shows all possible translations

tedbow’s picture

Whoops forgot to attach the patch.

I think Usability is kind of underselling it, no?

I think a big part of the issue is usability. Even if we can figure out how to surface all the overridden config if the user is thoroughly confused which I think could happen it really is not solving the problem.

I'd think incompatibility with Config Translation makes it "Outside In-critical"?

I would say yes. Getting to work with config overrides in general is longer term goal.

Status: Needs review » Needs work

The last submitted patch, 16: 2815709-16.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Wim Leers’s picture

#15 Nice! I think that works. Because there's basically nothing to configure for the Search block, I was fairly confused by what I saw in the video, simply because the majority of the UI elements were not for configuring the Search block, but for its translations. But I think it'll be clearer for e.g. the Menu block.

#16

I think a big part of the issue is usability. Even if we can figure out how to surface all the overridden config if the user is thoroughly confused which I think could happen it really is not solving the problem.

Good point!

Gábor Hojtsy’s picture

I believe the settings tray is supposed to let you edit things with some more complexity, let's say some pulled-out pieces of views. Grabbing the config translation form does not work in that complexity that also needs fully custom forms for this case. (A views config translation form is huge). Even for the simple search block case, you get a lot of cruft in the translation form, eg. the non-editable original title that is there for the translation use case (and if the viewport allows is formatted normally in a separate column). It also let's you edit the original text here, which is not true for any other thing in Drupal. What else in Drupal core lets you edit multiple variants of a thing at once? OTOH the translation would not have features like a toggle for displaying the title, so I assume that is why both forms are displayed.

Anyway, overall I think wider feedback on this would be essential rather than trying to figure out what is the primary target to shoot for between the three Acquians of us Wim Leers, Ted Bowman and myself. Also even if that decision lands at translations only, it would need a more serious look at how does that look like, how would editing some of the config details that are not translations intermix with the things that are translations, etc. Also if there are more overrides that are not translations, this would still present you an editing UI that looks like you are editing something that is not what you see even with translation support, and there was/is no solution to let you know in that case what is the "error".

I am more concerned of a half baked implementation now than missing this feature initially. Drupal 8 can introduce new features but also needs to be backwards compatible with what has been released as stable/beta/rc.

Version: 8.3.x-dev » 8.4.x-dev

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.

tedbow’s picture

Component: outside_in.module » settings_tray.module

Changing to new settings_tray.module component. @drpal thanks for script help! :)

tedbow’s picture

Issue tags: +Needs reroll
jofitz’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
9.17 KB

Re-rolled.

jofitz’s picture

Remove redundant code.

jofitz’s picture

FileSize
8.73 KB

Corrections to patch in #24.

The last submitted patch, 24: 2815709-24.patch, failed testing. View results

The last submitted patch, 23: 2815709-23.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 25: 2815709-25.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jofitz’s picture

Status: Needs work » Needs review
FileSize
2.26 KB
10.28 KB
  • Update tests to reflect changes in code.
  • Correct coding standards error.

Status: Needs review » Needs work

The last submitted patch, 29: 2815709-29.patch, failed testing. View results

tedbow’s picture

Version: 8.4.x-dev » 8.5.x-dev
Status: Needs work » Needs review
FileSize
10.29 KB

@Jo Fitzgerald thanks for these patches.

Here is patch that completes the reroll against 8.5.x

Status: Needs review » Needs work

The last submitted patch, 31: 2815709-31.patch, failed testing. View results

yogeshmpawar’s picture

Assigned: Unassigned » yogeshmpawar
yogeshmpawar’s picture

Assigned: yogeshmpawar » Unassigned
Status: Needs work » Needs review
FileSize
10.29 KB

Added patch for 8.4.x branch because previous patch failed to apply.

Status: Needs review » Needs work

The last submitted patch, 34: settings_tray-2815709-34.patch, failed testing. View results

yogeshmpawar’s picture

Status: Needs work » Needs review
FileSize
10.29 KB

Added updated patch because previous patch having a syntax error.

xjm’s picture

Ouchy, this is probably a must-have.

tedbow’s picture

@Yogesh Pawar thanks for the patches. but first we need to work on the 8.5.x patch and then after that is committed we will work on back porting to 8.4.x(if applicable)

@xjm ok. will work on it.

I chatted with @xjm and she said first pass for this issue may be:

  1. check if a block has overrides, translations or others.
  2. If it does provide message "this block can't be edit because of overrides"(or something) and don't show the form
  3. Provide a link to the advanced configuration form.

This maybe should be broken out into a separate issue with this issue remaining to determine if we can provide another solution that allows showing the form.

tedbow’s picture

@xjm I realize there was 1 key point I forgot to mention when we chatted about the issue.

I created #2919373: Prevent Settings Tray functionality for blocks that have configuration overrides but

In #10 @Wim Leers explained how it is possible to determine which modules provide configuration overrides. His suggestion was if the only language overrides then handle that.

UPDATE: chatted with @xjm again, she thinks #2919373: Prevent Settings Tray functionality for blocks that have configuration overrides is the way to go first then maybe look at the language only situation

Wim Leers’s picture

Title: Settings tray conceptually incompatible with configuration overrides such as translations » [PP-1] Settings tray conceptually incompatible with configuration overrides such as translations
Status: Needs review » Postponed
Related issues: +#2919373: Prevent Settings Tray functionality for blocks that have configuration overrides

#39 says #2919373: Prevent Settings Tray functionality for blocks that have configuration overrides should happen first, then this should happen. Updating issue status to reflect that.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.

tedbow’s picture

Title: [PP-1] Settings tray conceptually incompatible with configuration overrides such as translations » Settings tray conceptually incompatible with configuration overrides such as translations
Status: Postponed » Needs work

#2919373: Prevent Settings Tray functionality for blocks that have configuration overrides was fixed!

So now back to the broader issue.

As part of #2919373 #2923004: Add method to check if any overrides are applied to \Drupal\Core\Config\Config was fixed which added \Drupal\Core\Config\Config::hasOverrides()

Although ::hasOverrides() was good enough to prevent showing the "Quick edit" links for blocks where they have config overrides and to not show related config form elements such as menu and site name when that configuration has overrides.

But for this issue I don't think ::hasOverrides() is enough. For instance if a block label is overridden because of config_translation we don't have good way to tell if the block label is overridden also by another config override for instance by settings.php. This is especially complicated because the other configuration could actually have same value for label .

Originally in #2923004 getOverrides() which would actually returns the actual overrides but it wasn't completely clear how that would work.

but we should probably open up a follow up to create getOverrides() or something like it

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

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.

Version: 8.9.x-dev » 9.1.x-dev

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.

Version: 9.1.x-dev » 9.2.x-dev

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.

Version: 9.2.x-dev » 9.3.x-dev

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.

Version: 9.3.x-dev » 9.4.x-dev

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.

Version: 9.4.x-dev » 9.5.x-dev

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.

Version: 9.5.x-dev » 10.1.x-dev

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.

Version: 10.1.x-dev » 11.x-dev

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 the 11.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.