New option for Views page displays to use the admin theme

Ideally we would have an update path to set those default values.

+++ b/core/modules/views/src/Plugin/views/display/Page.php
@@ -217,6 +218,12 @@ public function optionsSummary(&$categories, &$options) {
+    $options['admin_theme'] = [
+      'category' => 'page',
+      'title' => $this->t('Use admin theme'),
+      'value' => $this->getOption('admin_theme') ? 'Yes' : 'No',
+    ];
   }

@@ -429,6 +436,14 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
+      case 'admin_theme':
+        $form['admin_theme'] = [
+          '#type' => 'checkbox',
+          '#title' => $this->t('Use admin theme'),
+          '#description' => $this->t('If checked, the admin theme will be used to display this page.'),
+          '#default_value' => $this->getOption('admin_theme'),
+        ];
+        break;

This is kind of an advanced option so I was wondering whether the option could be part of an existing form. Any thoughts on that?

Yes I'm thinking on this, but how? Probably the existing views that have page displays with admin routes are set through the code. In this case the only place where this info is stored is the {router} table. Should we:

Scan all the views for page displays
Then, for each, load the corresponding route
Extract the _admin_route value from there
Store it in the view

Nope. Its totally enough to resave the views ..., but note, there should be some code in \Drupal\views\Plugin\views\display\Page::alterRoutes to copy over previously existing admin route information.

Initially, I thought that should be together with the path/route. But that one is a simple option, so I didn't want to touch that.

I think yeah showing it on the path link is a good idea. Is that advanced that I won't bother to not show anything about it in the summary.

Well its about a 100% complete configuration, rather than relying on the default fallback.

... Adding the needs usability review ...

Interesting feature, I can imagine this be useful if your building operational views for authors who don't have admin access. Could you elaborate on the use case?

A few concerns:

• What if I enter a path with /admin in it - does it automatically check the box?
• What if I disable admin theme on a path using /admin?
• The whole description is useless.

@claudiu.cristea
Well, now its also a bit confusing, because the user might think that they can disable the admin theme via the UI, but well, they can't

Right, but this time we expose this behaviour even within just core.

+++ b/core/modules/views/views.post_update.php
@@ -203,5 +203,22 @@ function views_post_update_serializer_dependencies() {
+function views_post_update_admin_theme() {
+  $config_factory = \Drupal::configFactory();
+  foreach ($config_factory->listAll('views.view.') as $name) {
+    $view = $config_factory->getEditable($name);
+    foreach ($view->get('display') as $display_id => $display) {
+      // Deal only with page displays.
+      if ($display['display_plugin'] == 'page') {
+        $trail = "display.$display_id.display_options.admin_theme";
+        $view->set($trail, NULL)->save();
+      }
+    }
+  }

Should we respect value potentially set already by something else?

I'd like to see a feature like this go in. Not sure why there are two issues for it, though. I mean, I get that you want the option in and then build a UI for it in a follow-up, but the whole package shouldn't be that hard to get right in one go.

The way I see it, there are only four scenarios. Well two, really, but each has an on and off state:

1. Nothing alters the route, so the checkbox is the only control - good.
2. Something alters the route, so the checkbox may not represent the real effect - bad.

So the main thing we need to fix is the notion that an 'off' checkbox for an altered route would somehow disable the admin theme, which it won't. There are a few ways to go about this.

1. Turn the checkbox into a radio element with a clear description.

The label being "Admin theme behavior" and the options being "Inherit" and "Enforced". The description could state:

Choose whether this page will always use the admin theme, provided the user has access to see it, or if it will only use the admin theme when flagged as such through code (e.g.: paths starting with /admin).

Choosing "Enforced" would add the text between parentheses as shown in the screenshot in the other issue.

2. Actually make the checkbox trump all other route alterations

Using a route subscriber with a heavy weight, we could make the checkbox the ultimate admin theme defining mechanism. This would "break" core and other modules' views unless we write an update hook that figures out whether or not the admin theme is enabled for every individual view. And then there's still the issue of "default" views shipped by modules (config/install and config/optional) needing to be updated in their YAML files.

My preference goes to option 1 as it's more of a 'soft' flag. Introducing a 'hard' flag (option 2) could be done in D9.

P. S.: Loving the screenshots in the follow-up.

The last patch, new_option_for_views-2719797-1.patch, isn't working for 8.3.4. I tried re-rolling it, but it still doesn't seem to be working. Is there any chance someone can provide an updated patch for 8.3.4?

Thanks,
April

This solution helped me: https://www.computerminds.co.uk/drupal-code/drupal-8-views-how-set-admin....

I've given this some thought and I feel we should not introduce the three_way data type for this patch. It's currently serving as a placeholder because we're not including a UI in this issue yet. Once you add a UI option, how will you distinguish between NULL and FALSE with a checkbox? You'd need radio buttons where one maps to NULL, which I'd like to avoid.

So why don't we do the following:

• Keep the patch but change the admin_theme config key to boolean and update all existing views to start as FALSE.
• Immediately add a UI checkbox as shown in the other patch.
• If the checkbox is unchecked (FALSE), nothing happens.
• If the checkbox is checked (TRUE), then the route is flagged as an admin route.

The checkbox does not have to mean "do not show in the admin theme" when left unchecked. Instead, we could phrase it so the option becomes: "Default behavior" vs "Enforce admin path".

As a site-builder I'd expect this option to exist - it being missing is quite unfortunate.

I agree that the usability issue of /admin being always admin routes can be solved by simply phrasing the checkbox like the following:

"Enforce this path to be displayed in the admin theme"
Description: Note that paths starting with /admin/ are always shown in the admin theme.

This should explain everything and even document the so-far undocumented behavior, this it's even an UX improvement! ;)

I re-rolled the patch from #16 and improved the labelling as suggested. The patch applies to 8.3.x and 8.4.x as well.

@fago++ I really like where this is going!

\Drupal\views\Plugin\views\display\Page::getRoute got added in #2449143: REST views specify HTML as a possible request format, so if there is a "regular" HTML view on the same path, it will serve JSON, so this fatals when applied.

+++ b/core/modules/views/src/Plugin/views/display/Page.php
--- /dev/null
+++ b/core/modules/views/src/Tests/Update/AdminThemeOptionUpdateTest.php

+++ b/core/modules/views/src/Tests/Update/AdminThemeOptionUpdateTest.php
@@ -0,0 +1,43 @@
+use Drupal\system\Tests\Update\UpdatePathTestBase;
...
+class AdminThemeOptionUpdateTest extends UpdatePathTestBase {

Also, can we update this to extend \Drupal\FunctionalTests\Update\UpdatePathTestBase please (and move it to the right dir)

+++ b/core/modules/views/src/Plugin/views/display/Page.php
@@ -430,6 +435,14 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
+        $form['admin_theme'] = [

Can we rename the key as well, so it clear setting it to false might cause it to in the admin theme as well?

Really nice feature, and RTBC now in my book.

Great patch, loving it!

Minor nitpick:

+    // Check that enforce_admin_theme option is TRUE.
+    $this->assertFalse($options['enforce_admin_theme']);

That should read FALSE instead of TRUE, as indicated by the actual update hook.

No worries, thank you for working on it :)

+++ b/core/modules/views/views.post_update.php
@@ -213,3 +213,20 @@ function views_post_update_revision_metadata_fields() {
+    foreach ($view->get('display') as $display_id => $display) {
+      // Deal only with page displays.
+      if ($display['display_plugin'] == 'page') {
+        $trail = "display.$display_id.display_options.enforce_admin_theme";
+        $view->set($trail, FALSE)->save();
+      }
+    }

Can we ensure to not save the view multiple times when there are multiple page displays?

This is a cool idea for a feature! Looks like it has signoff from a(nother) Views maintainer already.

Here's what this looks like in the user interface (edit -- having some issues with file attachments; I'll try again shortly...)

1. +++ b/core/modules/views/config/schema/views.display.schema.yml
   @@ -68,6 +68,9 @@ views.display.page:
   +    enforce_admin_theme:
   +      type: boolean
   +      label: 'Enforce admin theme'
   

   I think "enforce admin theme" has undesirable connotations. A better choice might be "Always use admin theme".

2. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -218,6 +219,10 @@ public function optionsSummary(&$categories, &$options) {
   +      $options['path']['value'] .= ' ' . $this->t('(using admin theme)');
   

   In our UI text standard, "Using" should be capitalized here.

3. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -430,6 +435,14 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +          '#title' => $this->t('Enforce this page to be displayed in the admin theme'),
   

   This is a lot of words for the checkbox value. I think we could just change this title to "Always use admin theme" for better readability.

4. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -430,6 +435,14 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +          '#description' => $this->t('Paths starting with /admin/ are always shown in the admin theme. If checked, pages on other paths are shown in the admin theme as well.'),
   

   I would reduce this text to "Paths starting with /admin/ use the admin theme even when this is not set."

@claudiu.cristea sure!

All feedback from #48/#49 has been addressed. From a code standpoint this is RTBC again.

But I agree we need the usability review for the current proposed changed before moving forward. So leaving at 'needs review' for that.

I did a usability review wearing my site builder hat.

1. ✓ The UI is placed in a location which makes sense. I immediately found it where I expected it to be.
2. ✓ I find it user friendly that the overview shows that my path is going to be shown in the admin theme. I can easily see this at a glance without having to open the UI. I noticed that the path is shortened with an ellipsis to make room for the "Using admin theme" text, but this doesn't bother me at all. I can still see the start of the path and this gives me enough confidence that the page is actually using a path, and that I can open it to verify the full path in case I need to verify the path or change it. The path is not something that changes often so I trust it.
3. ✖ I am confused by the wording "Always use admin theme". Does this mean that when this is checked that the admin theme is "always" used, but when it is unchecked it is "sometimes" used? I interpret this as "maybe it is used, maybe not?". It would be clearer to me if this said "Display page using admin theme" or "Use the admin theme" instead.
4. ✖ The description helps to clarify it but, depending on my experience level, the example path might be a source of confusion. It currently is "/admin/" (without quotes) which starts with a slash. However the other examples do not start with a slash. Am I supposed to prefix the paths with a slash or not? It would be clearer if this did not start with a slash and was enclosed in quotes.
5. ✖ Lastly, I think the description would be even clearer if the ending "even when this is not set." was replaced with "even when this option is not checked."

Fair points Pieter, thanks for the review.

Re .3: The checkbox used to say "Enforce this page to be displayed in the admin theme", but that was changed to the current label following #49.3. A final version that everyone can agree upon would be nice :)

Thanks everyone. The usability review in #15 wasn't based on the latest version of the patch. Since @pfrenssen had other suggestions about the strings, let's get another usability review now.

The other screenshots were kind of small, so I added these:

We reviewed this on the weekly UX meeting.

We had several questions. As usual, we did not have time to review all the comments, so some of these may already have been discussed.

Discoverability

If a site administrator does not know where to look for it, then this checkbox is hard to find. Often, the administrator sets the path when first creating the view. Then there is no reason to edit it later, exposing this option.

It would be easier to find this option if it were added as one of the Page settings: Path, Menu, Access, and ... Admin? Clicking on the new option would bring up a page or modal with a more complete explanation.

Other effects

The path works by setting the _admin_route option on the route. This is checked by AdminContext::isAdminRoute(), which has effects other than setting the theme. For example, quickedit_page_attachments() will not enable in-place editing for an admin page.

So, is this issue about creating "admin pages" or is it about using the admin theme? If it is about "admin pages", then we need to be clearer about the effect of selecting the option. If it is about using a different theme, then why not present a choice of all enabled themes (as well as "site default" and "admin theme")? Either way, you can make good use of the extra space you get by making this a separate option under Page settings.

Something like this:

Reroll

Hi everyone,

I don't know if I am the only one with the following error when running db updates:
[error] Placeholders must have a trailing [] if they are to be expanded with an array of values.
[error] Update failed: views_post_update_enforce_admin_theme

Patch no longer applies to 8.6.4. Assuming that means it also won't apply to 8.7 anymore either.

Nope, it does still apply. Just some composer weirdness. Nvm.

Also, we've been using it since September and everything works fine.

+++ b/core/modules/views/views.post_update.php
@@ -366,3 +366,25 @@ function views_post_update_table_display_cache_max_age(&$sandbox = NULL) {
+  $config_factory = \Drupal::configFactory();
+  foreach ($config_factory->listAll('views.view.') as $name) {
+    $view = $config_factory->getEditable($name);
+    $changed = FALSE;
+    foreach ($view->get('display') as $display_id => $display) {
+      // Deal only with page displays.
+      if ($display['display_plugin'] === 'page') {
+        $trail = "display.$display_id.display_options.always_use_admin_theme";
+        $view->set($trail, FALSE)->save();
+        $changed = TRUE;
+      }
+    }
+    if ($changed) {
+      $view->save();
+    }
+  }

This should use the config entity updater. Does it it need to set the value or can it just do a save and the default value (hopefully FALSE) be completed?

Also manual testing shows me that I'm not sure that the update path is necessary. If I:

1. Install standard
2. Apply the patch
3. Go to a view that has a page display - for example admin/content and change the title of the view and press save
4. The always_use_admin_theme setting is not set.

It is only set if I invoke the dialogue to make the change. Therefore I think we can skip the update path and rely on the default value. This is also true for new views created via the wizard.

Also we already have a place in core where we can switch between admin and other themes so I think we should use that as inspiration. See the /admin/appearance page - all the way at the bottom:

I think we need to have a checkbox that says "Use administration theme" we also need to add similar text that this will impact who can see the view due to the permission to use the admin theme. Also if the path is set to something like admin/blah this option should be disabled and checked because it will be using the admin theme.

Also the summary label of Admin page: no makes no sense when the page is admin/content because this is clearly an admin page. See screenshot:

I think this page needs a bit more UI work to make it consistent. Also if we do this I think the setting should be use_admin_theme

This should use the config entity updater. Does it it need to set the value or can it just do a save and the default value (hopefully FALSE) be completed?

If you're simply updating the config,which seems to be the case, I'd say a hook_update_N is in order. If you want to actually use the entity API to save the views, then it should be a post-update hook that saves the entities, like Alex said.

Also manual testing shows me that I'm not sure that the update path is necessary. If I:

1. Install standard
2. Apply the patch
3. Go to a view that has a page display - for example admin/content and change the title of the view and press save
4. The always_use_admin_theme setting is not set.

It is only set if I invoke the dialogue to make the change. Therefore I think we can skip the update path and rely on the default value. This is also true for new views created via the wizard.

If this is intended, then yeah: You can skip the update path altogether.

On a sidenote: Wouldn't it be cleaner for the key to always be present in the config export, though? Or is that already the case? From your comment above, it seems to be missing when set to FALSE as the default (i.e. UI was not saved).

Patch rerolled for 8.8.x.

As suggested on #73 I updated the patch to
- use the config name use_admin_theme instead of always_use_admin_theme,
- improved (I hope) some texts to be more consistent with pre-existing options in Drupal
- mark the summary label to "Yes (Forced)" when the display path starts with /admin/...

Herg, uploaded a patch with a typo, ignore patch from #76!

#77 is good to go and addresses the feedback from #73. Have manually tested it, and agree that there's no need for an upgrade path.

Suscribe to this option, because actually I can't set admin theme for a Views Page (Manage Content page display) for users, the original page is served with admin theme...

Why are you changing the status, if the test failed on the patch.
I think no maintainer wil commit it, if he/she sees a failed test without any explanation.

Agree with #84

Fixed failed test and re-rolled onto 9.1 branch

Thanks @aleevas!

1. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +238,30 @@ public function optionsSummary(&$categories, &$options) {
   +    /*
   +     * If the views' dispaly path starts with 'admin/' the page will be
   +     * rendered with the Administration theme regardless of the
   +     * 'use_admin_theme' option.
   +     * Therefore we need to set the summary message to reflect this.
   +     */
   +    $display_path = $this->getOption('path');
   +    if (!empty($display_path) && 0 === stripos($display_path, 'admin/')) {
   

   Should we add a global option to views.settings.yml (/admin/structure/views/settings accordingly) for the 'admin/' thing?, for example:

   [✅] Force using the admin theme if the path starts with [admin/].
   

   Furthermore, what if users want an alternative path to be forced. Such as administration/, guan-li-yuan/ (Guan li yuan in Chinese pinyin means admin), or drupal/admin/

    [✅] Force using the admin theme if the path starts with [drupal/admin/].
   

   So, two config keys might be considered, for example:

   1. force_admin_theme_by_path: (default to TRUE)
   2. force_admin_theme_path: (default to /admin)

2. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +238,30 @@ public function optionsSummary(&$categories, &$options) {
   +    $display_path = $this->getOption('path');
   +    if (!empty($display_path) && 0 === stripos($display_path, 'admin/')) {
   
   @@ -442,6 +474,14 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +      case 'use_admin_theme':
   +        $form['use_admin_theme'] = [
   +          '#type' => 'checkbox',
   +          '#title' => $this->t('Use the administration theme'),
   

   Moreover:

   // Pseudocode.
   if (force_admin_theme_by_path === TRUE && $display_path matches $force_admin_theme_path ) {
     THEN, $form['use_admin_theme'] should be DISABLED, instead of telling people that you can check this or not, but whatever you do here won't take any effects.
   }
   

3. The last, should a hook or event be considered to allow contributed module(s) altering the default start with logic of matching the display path. e.g. what if they want the matching logic be contains string foobar etc.

Thanks everybody for working on this!

#87.1 the forced use for the admin theme on a /admin path doesn't come from Views, so we shouldn't add an option to Views to do that. Even if we want to do that, I'd say this is out of scope for this issue. Same goes for the second option, nice idea, but out of scope here, we can look at that in a follow up.
#87.2 Good idea, lets disable the checkbox on forced paths
#87.3 Also out of scope, we can look at that in a follow up

1. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -93,6 +93,13 @@ protected function getRoute($view_id, $display_id) {
   +    // Add the _admin_route option only if use_admin_theme display option
   +    // is TRUE. Otherwise, let other modules or alters to make a decision.
   +    // @see \Drupal\system\EventSubscriber\AdminRouteSubscriber::alterRoutes()
   

   This comment doesn't really add much value, do we need this? Seems pretty obvious that you only set this option when the right setting was set in the View

2. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +238,30 @@ public function optionsSummary(&$categories, &$options) {
   +    /*
   +     * If the views' dispaly path starts with 'admin/' the page will be
   +     * rendered with the Administration theme regardless of the
   +     * 'use_admin_theme' option.
   +     * Therefore we need to set the summary message to reflect this.
   +     */
   

   "the views' dispaly" can just be "the display".
   I don't think these need to be two sentences, but one.
   For all inline comments (even multi line) we use // not /* */

3. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +238,30 @@ public function optionsSummary(&$categories, &$options) {
   +    if (!empty($display_path) && 0 === stripos($display_path, 'admin/')) {
   

   0 === stripos is a yoda statement, we try to avoid those for readability

+++ b/core/modules/views/src/Plugin/views/display/Page.php
@@ -230,6 +238,30 @@ public function optionsSummary(&$categories, &$options) {
+    if (!empty($display_path) && 0 === stripos($display_path, 'admin/')) {
+      $admin_theme_text = $this->t("Yes (Forced by path)");
+    }
+    elseif ($this->getOption('use_admin_theme')) {
+      $admin_theme_text = $this->t("Yes");
+    }
+    else {
+      $admin_theme_text = $this->t("No");
+    }

This logic needs test coverage

Hi @Lendude,

I have tried fixing your points.

Thanks

Thanks, @Lendude! agreed with your reply to #87.

1. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +235,28 @@ public function optionsSummary(&$categories, &$options) {
   +    // If the display path starts with 'admin/' the page will be
   +    // rendered with the Administration theme regardless of the
   +    // 'use_admin_theme' option therefore, we need to set the summary message ¶
   +    // to reflect this.    ¶
   

   Thanks @Saurabh_sgh! No exceed 80 chars, great! But wrapped too early, and unexpected tail whitespaces.

2. #87.2 Good idea, lets disable the checkbox on forced paths

   #87.2 has not been addressed yet.

Hi @jungle,

I have addressed all your points in this patch.

Thanks

Thank you, @Saurabh_sgh!

1. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -230,6 +235,28 @@ public function optionsSummary(&$categories, &$options) {
   +    // If the display path starts with 'admin/' the page will be
   +    // rendered with the Administration theme regardless of the
   

   Should be adjusted further. "rendered with" wrapped too early as well.

2. +++ b/core/modules/views/config/schema/views.display.schema.yml
   index 3816ee179f..0c2e3a7762 100644
   --- a/core/modules/views/src/Plugin/views/display/Page.php
   
   +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -442,6 +469,18 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +          '#type' => 'checkbox',
   ...
   +          $form['use_admin_theme']['#attributes'] = array('disabled' => 'disabled');
   

   Meanwhile, it should be auto-checked if the display path starts with admin/

3. +++ b/core/modules/views/tests/src/Functional/Plugin/DisplayPageWebTest.php
   @@ -152,6 +152,37 @@ public function testPagePaths() {
   +  public function testAdminTheme() {
   

   Needs updating the test to cover the new change.

Hi @jungle

I have addressed your point except 96.3, I don't think there is change required in that part. If you still feel change is required, please guide me so that i could change it as per your suggestions.

Thanks

Thanks, @Saurabh_sgh! Perhaps, the official doc might help, see PHPUnit in Drupal.

1. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -224,12 +229,33 @@ public function optionsSummary(&$categories, &$options) {
   +    // This adds a 'Settings' link to the style_options setting if the style has
   +    //  options.
   

   We shouldn't be updating this comment, since it is unrelated to the patch.

2. +++ b/core/modules/views/src/Plugin/views/display/Page.php
   @@ -224,12 +229,33 @@ public function optionsSummary(&$categories, &$options) {
   +    if (!empty($display_path) && stripos($display_path, 'admin/') === 0) {
   ...
   +    elseif ($this->getOption('use_admin_theme')) {
   ...
   +    else {
   

   And the change we are referring to, is that we need to update the test coverage to cover this if/else piece of logic

Thanks @Lendude, +1 to #100.1, did not realize it.

@saurabh-2k17 : Patch#98 is failing on applying it for drupal 9.1. Needs re-roll of the patch.

Moving it to "Needs Work" for patch reroll

Re-rolled the patch and as per #100.1 kept comment as it was before.

Patch#106 is not working on drupal 9 as per expectation:
1. Check box should be labelled as "Always use admin theme"
2. Helper text for check box should be "Paths starting with "admin/" use the admin theme even this option is not checked"
3. On checking the box "Use the administration theme", Default theme is not appearing on the page.

Screenshots are attached.

Steps:
1. Visit /admin/structure/views
2. Open view page of any content
3. Click on "Admin page | No" under Page Settings.
4. A dialog box will appear with a checkbox
5. Click on checkbox and then on apply

Moving it to "Needs work" for fixing of issues on comment #107

Updated the patch as per #107

Tested patch #109 for the issues mentioned in comment #107. All the issues are covered in this updated patch and working fine. @claudiu.cristea- +1 for RTBC.
Screenshots are attached.

#109 working for me as well
RTBC + 1

I've tested the patch in #109. It applied cleanly but I found it would be more meaningful when we change the text "even this option is not checked" in,

to "even when this option is not checked" as said in #56. i.e,

Also, there are some coding standard issues,

So, I've corrected all of this in my patch. Hereby attaching the patch and interdiff.

Please review. Thanks!

I think we should be cautious about disabling this option on paths that start with /admin. Yes these use the admin theme by default, but modules and sites could customise that behavior.

I'd suggest simply having the description saying something like "By default paths beginning with /admin/ will use the admin theme without regard to this setting.".

I did try #113 and it is OK.
RTBC + 1

RTBC+1 here too. We have it under testing for about a week, we will be deploying to production very very soon.

The patch #113 applies and works as expected on a Drupal 9.0.11 site. Great improvement. +1 to RTBC

The patch no longer applies.

This seems potentially useful, but some of the wording could use some help IMO, tagging for usability review.

+++ b/core/modules/views/src/Plugin/views/display/Page.php
@@ -232,6 +237,27 @@ public function optionsSummary(&$categories, &$options) {
+    $display_path = $this->getOption('path');
+    if (!empty($display_path) && stripos($display_path, 'admin/') === 0) {
+      $admin_theme_text = $this->t("Yes (Forced by path)");
+    }

Can we use something like 'Yes (admin path)' or something instead of 'forced by path'? Definitely adds some complexity having to deal with both cases.

Rerolled for 9.2.x and made the suggested change in #120. Please review.

Patch #120 works for me (9.1.5)

Thanks!!

Usability Review

We discussed this issue at #3210553: Drupal Usability Meeting 2021-04-30.

1. @keopx, an issue should be marked "Reviewed & tested by the community" (RTBC) only when it has been both reviewed and tested. From your comment, it seems that you tested, but you did not say anything about review. In fact, the patch in #120 did not pass automated tests, so we have to set the status back to "Needs work" (NW).

   The message "Custom Commands Failed" usually means that there are problems found in static analysis: spelling errors, coding standards problems, etc. Look at the test results for a full report.

2. The modal window has the message

   Paths starting with "admin/" use the admin theme even this option is not checked.

   That should be "… even when this option is not checked." The screenshot in the issue summary shows the correct text.

3. The consensus at the usability meeting was that "Admin page" is misleading. It suggests access restrictions as well as a theme. (As I said in #61, there are some effects other than changing the theme, but access restriction is not one of them.) I have browsed the comments (not read all of them) and I am not sure when the text changes from "Always use admin theme" (as in #49, for example) to "Admin page". Was there a full discussion? I do see discussion in #56-#58, settling on "Always use admin theme".

4. If a user does not have permission to use the admin theme, then the page will be shown in the site’s default theme. Perhaps the description in the modal window should mention this.

The change to 10.0.x was a gllitch. Back to 9.3.x. See #3211455: Bulk updates for the 9.2.x release cycle.

New patch with subtle changes:

• Added 'Administration theme' to the title of the popup - it otherwise just said 'Page:'
• Change 'Admin page' to 'Admin theme', that makes more sense indeed.
• Fix the description adding 'when'

I don't have any other strong opinions, so if someone decides on the working, let's go ahead :)

Fix errors

I have applied and tested the patch #126. it is showing the following changes done as per #125.

Added 'Administration theme' to the title of the popup - it otherwise just said 'Page:'

Change 'Admin page' to 'Admin theme', that makes more sense indeed.

Fix the description adding 'when'

Adding screenshots for reference.

Nice work folks. Patch #126 working well for me.

This is such a useful and great patch #126. I have applied at my job it is really useful I think I was waiting for it.

FWIW, the patch at #126 is working as expected and does a great service.
Marking as RTBC to help push this issue forward.

Tests are failing because of #3255749: Composer v2.2 prompts to authorize plugins

We need to do a little bit of work to not save the setting if the views path starts with admin/

Currently if I do this:

1. Install standard
2. Export configuration
3. Visit admin/structure/views/view/user_admin_people
4. Click on link Admin theme: Yes (admin path)
5. Press the apply button
6. Press the save button
7. Visit http://drupal8alt.test/admin/config/development/configuration

I'll see that the view has been updated to set use_admin_theme: false which is odd because it will use the admin theme. I think it shouldn't be saving the setting in this case.

Also if do this:

1. Install standard
2. Export configuration
3. Visit admin/structure/views/view/frontpage
4. Click on link Admin theme: No
5. Press the apply button
6. Press the save button
7. Visit http://drupal8alt.test/admin/config/development/configuration

I'll see a new use_admin_theme: false even though I've not really changed anything.

This brings into question whether we need an update path to add this to all views which leverage a page display. I think it might be best to not set this configuration when it is FALSE. it's not as if we're ever going to change the default behaviour. Not sure.

Rebased against 9.4.x and opened MR.

Uploading a patch against 9.3.x. The patch from #126 still applies to 9.2.x.

I tested this patch and it worked for me in Drupal 9.2.12

need to be update with drupal dev version
->set('admin', 'seven')
Drupal >= 9.4 admin theme is Claro

I have successfully applied patch #137 on drupal 9.2.22-dev and its working for me
Thanks @pfrenssen. Adding screenshots for the refrence

Moving to NW for
Can we get a tests-only patch with the full path also please to verify the tests work.
#141 needs to be addressed
And Bug found in last bullet of testing.

Manually tested

• Patch applies
• Cloned the content view.
• Changed URL to /test
• Verified page rendered with front end theme
• Updated view with checking new setting to use admin theme
• Verified page rendered with admin theme.
• Changed URL to /admin/test
• Verified checkbox is auto checked.
• Verified page rendered with admin theme.
• BUG: The URL of the page is now just /admin vs /admin/test

No more screenshots are needed.

Updated for #141.
Uploaded tests only patch

The bug mentioned above "BUG: The URL of the page is now just /admin vs /admin/test" may not actually be an issue it seems.

looks like working for me. :D

I have added a Test page view

The admin theme working on both URL

admin/test and test

can restrict if don't change URL with admin/?

https://www.drupal.org/files/issues/2022-09-20/2719797-144.patch - works for me.

#136 is still not addressed.

#144 no longer applies to Drupal 9.4.8, needs a re-roll

Created patch for 9.4.x by considering the comment of #152. Please review this patch and move it

Fixed CS error.

Fixed failed custom command for comment #154.

Removing credit for #153, #154, #155.

Patch #144 still applies cleanly to 9.5.x and 10.1.x so the follow up was not needed.

Will look into #136

@alexpott regarding #136 this something like you meant?

Applied patch #158 on drupal version 10.1.x-dev successfully and working fine.
Refer to screenshot.
Thank you

Great patch to avoid custom hacking, now we can easily apply admin theme.
+1 RTBC