When changes are made for all displays, mark all displays as changed

Could you describe what settings you changed?

Actually confirm - other display tabs does not indicates their "changed" state when changes applies to all displays.
This mostly about usability - when you change a some option and point to apply to all Displays so there's no visual indication on other displays that they got changes

posted twice

No need for usability review, you're just 100% right :)

Here is juat an idea how to fix that. This especially needs a test.

+++ b/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -8,6 +8,7 @@
+use Drupal\views\ViewUI;

This has moved.

And, it needs tests, so no reason to reroll until those are written.

Wrote a small test for this, and rerolled against the latest version.

Yes please!

So

So, the tests didn't make too much sense passing here :) I have fixed them up a bit and fixed the assertLinks.

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/DisplayTest.phpundefined
@@ -195,14 +195,16 @@ public function testChanged() {
+    $this->assertLink(t('Master'), 0, 'The master display is still marked as changed.');

Well Master will always exist (as there is a link available ont he site), but i guess it should not mark be as changed if we override it?

Rerolled and changed the assertion message in #14.

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/DisplayTest.phpundefined
@@ -182,4 +182,29 @@ public function testDisplayPluginsAlter() {
+    $this->assertLink(t('Master'), 0, 'The master display is not marked as changed.');

Let's better also add a $this->assertNoLink(t('Master*')) to be sure it is really not overridden

Looks good to me.

#16: drupal-1791352-16.patch queued for re-testing.

DisplayPluginBase::setOption() doesn't have access to the View UI object, we should pass it along when necessary.

Makes sense to me, IMO the code looks slightly cleaner like this, losing the instanceof stuff.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -2154,7 +2175,7 @@ public function submitOptionsForm(&$form, &$form_state) {
+            $this->setOption('access', $access, $form_state['view']);

What about using an actual setOptionUI which wraps setOption with that logic, to keep the api function as simple as possible.

Agreed.

Okay, done. No interdiff really, since it's a 100% different patch.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -935,6 +945,31 @@ public function setOption($option, $value) {
+  public function setOptionUI($option, $value, $view_ui) {

Just a small thing: use ViewUI in the signature

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -935,6 +945,31 @@ public function setOption($option, $value) {
+    if ($this->isDefaulted($option)) {

I guess it makes sense to describe what this function exactly does, because it is not easy to figure out.

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/DisplayTest.phpundefined
@@ -210,4 +210,29 @@ public function testDisplayAreas() {
+  public function testChanged() {

It's good to have a total different implementation but still have a way to keep the test.

#25: drupal-1791352-25.patch queued for re-testing.

#25: drupal-1791352-25.patch queued for re-testing.

Rerolled, patch was pretty out of date.

A small problem with big consequences.

Another rerole, with changing config() to use the container.

#32: drupal-1791352-32.patch queued for re-testing.

s/name/id

Perfect, thanks for fixing that.

Are there other places in the code base that explicitly reference views_ui? This seems a bit backwards.

Let's postpone that on #1919142: Convert Views UI AJAX forms to use FormInterface
The other way to fix this would be to directly call some function on the ViewUI directly in the submit functions.

per #39

Seems patch outdated

Rerolled patch #36.

Thank you @aerozeppelin for rerolling the patch! The patch for itself looks really solid. Here is just one question I have ...

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
@@ -2641,6 +2648,33 @@ public function getExtenders() {
+  public function setOptionUI($option, $value, ViewUI $view_ui) {

Does this method has to be public?

@dawehner thank you for reviewing the patch.

Does this method has to be public?

I agree making it public has no significance.

@aerozeppelin
Note: Drupal uses protected most of the time, let's just keep it that way.

Updates as per #51.

Thank you @aerozeppelin!

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
@@ -2641,6 +2648,33 @@ public function getExtenders() {
+  protected function setOptionUI($option, $value, ViewUI $view_ui) {

Given that we're adding a method this needs to be for 8.3.x only. Also it'd be nice to get at least one green run on #52

Looking at this change closer - is there anyway of doing this change in a way that does not bring views ui code into views code?

Rerolled the patch.

I tested on Drupal 9.4.x and was able to reproduce the problem.

This needs an issue summary update, see Write an issue summary for an existing issue for guidance.

An updated patch is needed and the #55 needs to be addressed.

Rerolled patch #57 against drupal 9.5.x

Rectified custom commands failed errors.

2731 | ERROR | [x] Expected 1 blank line after function; 0 found
2758 | ERROR | [x] Whitespace found at end of line

Rerolled patch #56 with Drupal 9.5.x

Verified patch #74 on 10.1.x-dev. Patch applied cleanly with several hunks.

However, after applying patch site breaks and gives "HTTP 500 error - This page isn't working" error. No page is accessible.
We need working patch for 10.1.