Layout Builder cannot be uninstalled while overrides exist; no easy way to revert all overrides

It's because one of your Basic page has a overriden layout which prevents Layout Builder from being uninstalled. You'll have to reset those nodes overriden layout (You can do that by going to that node's layout and choose Revert layout) or delete them.

Has anyone found a solution to this? I can't find a way to reset the layout as post #2 has suggested.

Hi Mihi-r

I also had the same issue, what i did was to untick the layout for a content type that i was trying to configure with layout builder, then cleared cache and uninstalled. it worked for me

This provides a VERY DANGEROUS BUTTON that will permanently remove all layout overrides for a given bundle.
It should probably be behind a confirmation form.

But in the meantime, it might help!

This patch *should* work for 8.5 and 8.6, but targeting for 8.7

Oops, empty patch.

This is made extra necessary by the fact that revisions hold layout data, so you have to revert layout for every revision (or delete every revision). A global remove custom layouts is a much better way to handle that.

Let's get some input from @DyanneNova on direction

Removing the checkbox (Allow each content item to have its layout customized.) on each contentypes Manage display unlocks the layoutfield and lets you uninstall the Layout Builder.
I had problems using/configuring the ief_table_view_mode Module after the Layout Builder was active.

As Comment #5 Lloyd_87 indicated, uncheck the "Allow each content item to have its layout customized." checkbox then uninstall the layout_builder module.

I stumbled over this problem quite by accident. It wasn't immediately obvious how to undo the changes that were made because even though I unchecked the checkbox, the changes that were made did not revert properly. It took uninstalling the layout_builder module to do that.

There is no easy solution. The "Allow each content item to have its layout customized." can't be unchecked. It says "You must revert all customized layouts of this display before you can disable this option."

WTH? Absolutely horrific dev experience. What am I missing?

I guess layouts is the new thing, but I prefer not to use it. Can't I just delete it?

That checkbox is unchecked by default. And is only disabled if you start using the functionality.
This issue is about making it easier to stop using once you choose to start using it.

This need some work from @DyanneNova, not ready for wider review.

I think the current patch is the right direction. We need a confirmation page after clicking Revert All to warn the user that they are irreversibly deleting data from all content of that type before they proceed.

Thanks!

Partially complete route addition.

Worked on / "complete":
- addition of a confirm form
- moving the actual revert process to the confirm form controller
- using DI for the services used in the revertAll method

Outstanding todos:
- finish wiring this up, hasn't been tested and I'm pretty sure it will just fall over in present state
- finish making this a dynamic route instead of a static one.
- run the reverts as a batch process.

Setting to NR for tests :)

@johndevman I think @ccasals left it at NW because it's a partial implementation. Give the testbots a break! :)

Tagging for the work done at DrupalCon

Before wrapping up the rest of this, some patch hygiene :)

Reroll of patch in #9
interdiff for the reroll
interdiff between reroll and #23

(thanks for pointing that out @tim!)

Triggering bots for tests.

So this patch "works" - the reversion takes place, the layout field is removed, and the display is set back to the default as it was before LB. However, there is still the message: `Fields pending deletion` when uninstalling Layout Builder is attempted - either through drush or the UI

Also includes a re-reroll of 2980299-revert-26-reroll.patch which was formatted incorrectly.

Patches flummox me, apparently.

This is looking really good! Thanks for continuing to push on this @ccasals.

Two pointers on things that have changed since 8.6:

1. +++ b/core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php
   @@ -197,4 +218,42 @@ class LayoutBuilderEntityViewDisplayForm extends EntityViewDisplayEditForm {
   +          /** @var \Drupal\layout_builder\OverridesSectionStorageInterface $section_storage */
   +          $section_storage = $section_storage_manager->loadFromStorageId('overrides', "$entity_type_id.$id");
   

   loadFromStorageId() is deprecated. You'll likely need to $entity_storage->loadMultiple($ids) and loop over the entities instead of the IDs, then pass them in as context.

   See \Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage::deriveContextsFromRoute() (the middle part after extractEntityFromRoute) for an example.

2. +++ b/core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php
   @@ -197,4 +218,42 @@ class LayoutBuilderEntityViewDisplayForm extends EntityViewDisplayEditForm {
   +          while ($section_storage->count()) {
   +              $section_storage->removeSection(0);
   +            }
   

   There's a new removeAllSections() method that can be used now

This patch:

1. Addresses both feedback items in #30
2. Made several changes to meet code standards. Code sniffer was able to identify most of them, but I want to explicity point out the one that code sniffer would not catch. @throws annotations should only be used when an exception is thrown in the method itself.
3. The "Revert All" button was going to the confirmation form for disabling layouts without reverting overrides. The dynamic route had to be changed to point to the confirmation form for reverting overrides.
4. Updated DefaultsSectionStorageTest so it passes
5. Renamed RevertAllConfirm to RevertAllConfirmForm so it follows form controller naming conventions.
6. Removed functionality from LayoutBuilderEntityViewDisplay that had been moved to RevertAllConfirmForm
7. Additional changes to RevertAllConfirmForm so it successfully removes overrides for the bundle.
8. Moved/added several methods that are likely to be reused into traits. The methods added to LayoutEntityHelperTrait are from a not-yet-committed patch for #3035992: Stale values can be displayed by the Layout Builder UI but are saved correctly
9. Changed the wording of the layout/override descriptions and Revert All button to make it more clear that they are related to one another, and that "Disable Layout Builder" does not mean disabling the module. There's probably a better way to phrase it, but it should be clear that when the layout/override buttons are disabled, the Revert All button should be used instead. Previously, the checkbox descriptions used the term "customized layouts", while the Revert All button used "layout overrides"

This still needs tests and (as mentioned in #22) the reverts should use a batch process.

Added tests and removed a bit of redundant code in LayoutEntityHelperTrait

I attempted to move the reverts to run in a batch process, but ran into errors when calling $override_section_storage->removeAllSections(); stating that the layout_builder__layout was not found despite it being present. My attempts were unsuccessful so they're not in this patch, and hoping I just overlooked something obvious that will be apparent once I've had a bit of distance (is something out deriving contexts from routes throwing this off?). If anyone has suggestions as to why this doesn't work in a batch process, happy to hear them:

 /* @var \Drupal\Core\Entity\FieldableEntityInterface $entity_with_override */
    foreach ($entities as $entity_with_override) {
      $override_section_storage = $this->getSectionStorageFromEntity($entity_with_override);
      $override_section_storage->removeAllSections();
      $override_section_storage->save();
      $this->layoutTempstoreRepository->delete($override_section_storage);
    }

Changed the reverts to a batch process, this is fully ready for review now.

Confirming, after testing around on this interesting issue I run into the issue scenario described in the summary.

After reverting all nodes I was able to disable Layout Builder for the content type. The checkboxes were greyed out before.

@ actual stable Drupal core release non of the patches applies atm here. Maybe sth wrong here on our side.

The patch in #36 applies cleanly to 8.9.x, 8.8.x, and 8.8.1

It has tests now, and needs review.

@tim.plunkett Are your sure with applying on 8.8.1 ? hm, I need to retest ...

Ok. For some reason the first attempts failed but after @tim.plunkett has stated that it should work, I re-tested the patch here on a fresh 8.8.1 with enabled Layout Builder for content type "page" and it worked. Thanks @tim.plunkett for confirming again that it should apply. No idea what went wrong before. Here the output. Testing 8.9.x and 8.8.x will follow.

git apply -v 2980299-36.patch
Checking patch core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php...
Checking patch core/modules/layout_builder/src/Form/RevertAllConfirmForm.php...
Checking patch core/modules/layout_builder/src/LayoutDisplayHelperTrait.php...
Checking patch core/modules/layout_builder/src/LayoutEntityHelperTrait.php...
Hunk #2 succeeded at 179 (offset 1 line).
Checking patch core/modules/layout_builder/src/Routing/LayoutBuilderRoutesTrait.php...
Checking patch core/modules/layout_builder/tests/src/Functional/LayoutBuilderUninstallTest.php...
Checking patch core/modules/layout_builder/tests/src/Unit/DefaultsSectionStorageTest.php...
Hunk #1 succeeded at 463 (offset 15 lines).
Hunk #2 succeeded at 541 (offset 15 lines).
Applied patch core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php cleanly.
Applied patch core/modules/layout_builder/src/Form/RevertAllConfirmForm.php cleanly.
Applied patch core/modules/layout_builder/src/LayoutDisplayHelperTrait.php cleanly.
Applied patch core/modules/layout_builder/src/LayoutEntityHelperTrait.php cleanly.
Applied patch core/modules/layout_builder/src/Routing/LayoutBuilderRoutesTrait.php cleanly.
Applied patch core/modules/layout_builder/tests/src/Functional/LayoutBuilderUninstallTest.php cleanly.
Applied patch core/modules/layout_builder/tests/src/Unit/DefaultsSectionStorageTest.php cleanly.

I can confirm that the patch applies cleanly and a new link is occuring under the Layout checkbox and that the user is now able to easely revert changes made and disable Layout Builder on Manage Display @ Drupal 8.8.1 and that the checkboxes are not greyed out no more.

But a possible follow up?

 drush cron
 [error]  InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/ContentEntityBase.php).
 [notice] Cron run completed.

extend/uninstall...

The following reason prevents Layout Builder from being uninstalled:
    Fields pending deletion

The field has been removed from the Manage DIsplay field management by reverting and unchecking Layout Builder and Custom Layout for each item, but it seems to have some left overs. Running cron several times did not remove it.

Possibly related?: #2861863: InvalidArgumentException "Field is unknown" in ContentEntityBase

Maybe appears only for projects with multiple languages installed?

I'd need a longer backscroll to understand that error.

@tim.plunkett: Yeah ...Maybe its cleaner to create a new issue? Because it has some other related issues and is another step in the whole problem than the one solved here in this issue. But still: they are more than related. I am even not sure if the patch has created this error and SWOD.

Let's say it the other way around: The patch here fixes the problem described in the issue headline and summary, yes, but after applying it, the error message from the closed core issue regarding unknown field in case of a multilanguage setup occures in the next step #2861863: InvalidArgumentException "Field is unknown" in ContentEntityBase And since this issue has been closed because of the overall problem, that it only describes a correct error message which has possible multiple reasons, I think it falls back to the individual core components and contrib modules causing it. Or our latest patch version here :/

I think it still needs work, since the patch causes errors and WSOD including this error reported on #2861863: InvalidArgumentException "Field is unknown" in ContentEntityBase

[error]  InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/ContentEntityBase.php).

The following reason prevents Layout Builder from being uninstalled:
    Fields pending deletion

Would love to read other reviews testing the patch on a multi-language setup.

I tried to provide steps to reproduce but the main issue does not appear no more in a fresh Drupal core installation from what I see here. EDIT: Ok we managed to reproduce the main issue from Summary and the secondary issue caused by the patch again. It seems to require more than 1 test node created. Here: Fresh Drupal installation on https://stm5e18fc4d9ca89-5noulyeqtcsebr896mg1ydo5gyzcotea.tugboat.qa Feel free to join the test (admin/admin) with Devel and Devel generate installed for creating mass content in different languages.

1. Install latest Drupal core stable with Devel.
2. Install/activate the following modules: Devel, Devel generate, Language, Content Translation, Layout Builder.
3. Enable second language and set it to default and move it on top.
4. Enable language and translation for content type "Article".
5. Create multiple nodes in different languages for "Article" via Devel generate.
6. Enable Layout for "Article" for different Displays like Full content, Teaser, etc. and activate "Allow each content item to have its layout customized." for Full content Display.
7. Create more multiple nodes in different languages for "Article" via Devel generate.
8. Try to find a way to manage to disable Layout Builder for Article to later uninstall Layout Builder.
9. If you can't manage to disable Layout for "Article" then apply the last patch of this issue queue and try it again.
10. After successfully disabling the Layout settings for Article try to uninstall Layout Builder again. It should be greyed out with message:
11. Fields pending deletion
12. Now try to flush caches and run cron via terminal. This should make the following error appear:
    

      [error]  InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/ContentEntityBase.php).
      The following reason prevents Layout Builder from being uninstalled:
          Fields pending deletion
      

13. Now try to delete a field in the content type "Article". It will cause WSOD with same error.

I know it would be tedious, but can you try the exact about but without Devel (aka only core modules)?
That would provide a clear set of steps we could use to write a core test case.

Makes sense... Done! Any reports or logs I should add?

New (corrected) reproduction steps done here:

1. Install latest Drupal core stable "Standart profile".
2. Install/activate the following modules: Language, Content Translation, Layout Builder.
3. Enable second language and set it to default and move it on top. (We used German).
4. Enable language and translation for content type "Article" with partially fields like Body and Title set as translatable.
5. Create multiple nodes in different languages for "Article" manually.
6. Enable Layout for "Article" for different Displays like Full content, Teaser, etc. and activate "Allow each content item to have its layout customized." for Full content Display.
7. Create more multiple nodes in different languages for "Article" manually (we did 2 before and 2 after in the 2 different languages).
8. Try to find a way to manage to disable Layout Builder for Article to later uninstall Layout Builder.
9. If you can't manage to disable Layout for "Article" then apply the last patch of this issue queue and try it again.
10. After successfully disabling the Layout settings for Article try to uninstall Layout Builder again. It should be greyed out with message:
11. Fields pending deletion
12. Now try to flush caches and run cron via terminal. This should make the following error appear:
    

      [error]  InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/ContentEntityBase.php).
      The following reason prevents Layout Builder from being uninstalled:
          Fields pending deletion
      

13. Now try to delete a field in the content type "Article". It will cause WSOD with same error.

Additionally we tested to revert all Layout changes made manually (without the patch applied) down back to the point where Layout builder is able to get disabled again on the content type (remove all custom layout settings manually) which works and does not cause the issue described in #46/#47. I will try to find time to read the patch code again... not sure where to look at yet at a glance since we have some dead lines here and some operational failuer (office off time) due to illness (many people suffer from a gastric flu virus over here atm). :/

Interestingly enough another issue appears here: old layout settings re-occure after enabling Layout builder again ... \0.0/ which makes no sense because we have removed them before disabling Layout builder ...

I followed the instructions and after step 10, I was able to uninstall the module. No "Fields pending deletion" message at all.

Ok, thanks for taking the time to report back, Tim.

Weird ... Yeah, so .... doing it again (one MORE time) all seems fine now and the patch works, creates the link to revert changes and makes it easy to disable and uninstall Layout Builder. +1 for the awesome work on this.

There is just something going on here, but we just don't know yet what it is. But maybe unrelated to this issue ... Where is the overlapping here in our 2 test scenarios causing the same issue? This should be the spot to look at from our side to make sure this issue here is not affected. I actually need only one more test running OK like yours and then I can compare. Let me try to test on another environment a fourth time.

#40-52 are unrelated to this issue. Please open a new one (or two) to discuss.

Rerolling #36 and making a few adjustments.

Inlined revertAll and moved the redirect to be part of the batch.
Done tweaking now.

drupal/web$ git apply -v 2980299-revertall-54.patch
Checking patch core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php...
Checking patch core/modules/layout_builder/src/Form/RevertAllConfirmForm.php...
Checking patch core/modules/layout_builder/src/LayoutDisplayHelperTrait.php...
Checking patch core/modules/layout_builder/src/Routing/LayoutBuilderRoutesTrait.php...
Checking patch core/modules/layout_builder/tests/src/Functional/LayoutBuilderUninstallTest.php...
Checking patch core/modules/layout_builder/tests/src/Unit/DefaultsSectionStorageTest.php...
Applied patch core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php cleanly.
Applied patch core/modules/layout_builder/src/Form/RevertAllConfirmForm.php cleanly.
Applied patch core/modules/layout_builder/src/LayoutDisplayHelperTrait.php cleanly.
Applied patch core/modules/layout_builder/src/Routing/LayoutBuilderRoutesTrait.php cleanly.
Applied patch core/modules/layout_builder/tests/src/Functional/LayoutBuilderUninstallTest.php cleanly.
Applied patch core/modules/layout_builder/tests/src/Unit/DefaultsSectionStorageTest.php cleanly.

+1 ... and I can confirm that an input link "Disable Layout Builder for this display and revert all layout overrides" occures under Layout options in Manage Display and clicking the link causes:

Are you sure you want to revert all layouts to the default layout for this display? Layout Builder will also be disabled for this display.

Confirming this choise will cause:

Layout Builder disabled and all overrides removed.

... and all layout changes made have been reverted and Layout Builder is disabled for this content type (unchecked). Uninstalling Layout Builder is now not greyed out no more (on extend/uninstall) and can be selected for uninstalling.

+1 and RTBC from me. Great work on this and a great and handsome addition!

Sorry for not having read every single answer.
I had a particular node blocking this function (Allow each content item to have its layout customized.).
I had no possibility of checking each one individually since there are more them 350 nodes under this content type.
I went to the database and under the table "node__layout_builder__layout", I managed to get the node ID of the offending.. node.
Opened using the dirty URL and reverted it to the standard layout, allowing me to uncheck the "Allow each content item to have its layout customized" option.

Just adding some info here in case it's helpful for identifying the issue. I am seeing this on cron run, specifically when the "Purges deleted Field API data" cron is run:

InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /mnt/www/html/mysitedev/docroot/core/lib/Drupal/Core/Entity/ContentEntityBase.php).

I'm using Drupal 8.8.4 and PHP 7.2.27. I did not try the patch. Currently, we are not seeing any critical behaviors like a WOSD, but it's possible we just haven't gone to the right page to see it.

#58 is that as a result of the patch?
It reads like something that would happen if someone manually removed the LB field in an attempt to solve this bug, but didn't correctly remove it and all references to it.

I can confirm Tim's assumption on #58 since we had that scenario already and it was indeed that case.

Sorry, it seems I made this ticket more confusing!

#58 is not a result of the patch submitted in #54. I wrote comment #58 thinking this ticket was to address the issue I was observing, and wanted to share that I had too had experienced this issue.

I applied the patch from #54 and still see my error in the cron log for "Purges deleted Field API data", which is:

InvalidArgumentException: Field layout_builder__layout is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 587 of /var/www/mysite/drupal/docroot/core/lib/Drupal/Core/Entity/ContentEntityBase.php).

However, suggesting that it has to do with the data structure on a content type prompted me to identify the problematic nodes, delete them, run the cron again and see if the error resolves. To do this I put a breakpoint on line 587 on docroot/core/lib/Drupal/Core/Entity/ContentEntityBase.php and then inspected $this->entityKeys which showed the bundle and revision ID. I went the long route of looking up the node the revision belongs to in MySQL select * from node_revision where vid=101;, then going to /node/{nid}/delete. I only had to repeat that twice because there were only 2 nodes that were causing the issue.

In conclusion, I don't know the original cause. Fortunately, I can easily re-create those 2 problematic nodes. It's unclear how this ticket relates to my problem (if at all). And, it's unclear if there is a solution that doesn't require data loss.

I'm unblocked -- thank you for the help!!

Looks like we need a reroll on D9. The 9.1.x test fails with:

ayout_builder.Drupal\Tests\layout_builder\Functional\LayoutBuilderUninstallTest
✗	
Drupal\Tests\layout_builder\Functional\LayoutBuilderUninstallTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-672.xml:
PHPunit Test failed to complete; Error: PHPUnit 8.5.3 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\layout_builder\Functional\LayoutBuilderUninstallTest
.                                                                   1 / 1 (100%)

Time: 19.28 seconds, Memory: 4.00 MB

OK (1 test, 23 assertions)

HTML output was generated

Remaining self deprecation notices (1)

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\layout_builder\Functional\LayoutBuilderUninstallTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

I don't think this issue ever had any usability review done, both of the functionality and of the UI text.

Drupal already providing an option for Disable Layout builder.
So I think this bach process only needs to consider reverting the customized layout.
So I have removed one line from the above patch.

Solved patch error

Fixed the deprecation notices.

Fixed the failing tests.

Patch applies and resolves issue on simplytest.me

1. Project drupal 9.1.2 Launch sandbox
2. Install/enable Layout Builder
3. On Article content type Manage Display /admin/structure/types/manage/article/display, enable: Use Layout Builder
4. enable: Allow each content item to have its layout customized.
5. Save
6. Create a node of type Article
7. customize layout /node/1/layout
8. Save layout
9. Go to /admin/structure/types/manage/article/display/default
10. Verify LB cannot be disabled
11. go to /admin/modules/uninstall
12. Verify LB cannot be uninstalled: The Layout Section field type is used in the following field: node.layout_builder__layout

With patch:

1. Project drupal 9.1.2, Add a patch (https://www.drupal.org/files/issues/2021-01-18/2980299-72.patch), & Launch sandbox
2. Repeat steps 2-9
3. Click on Disable LB for this display and revert all layout overrides
4. Confirm
5. Disable Use Layout Builder
6. go to /admin/modules/uninstall
7. Verify LB can be uninstalled

Since #65 hasn't been addressed yet seriously I fear to set this RTBC. Also #67 "Drupal already providing an option for Disable Layout builder." tells me this issue maybe needs more discussion? Do we "cross-fix" an issue from the wrong side here?

i fix it
1) go to /admin/structure/types
2) choose each content type
3) go to manage display and unselect layout builder.

From #76:

3) go to manage display and unselect layout builder.

That does not work if there are any layout builder overrides in place for that content type. If overrides exist, the checkboxes are grayed out:

This is the problem the issue is attempting to overcome.

This needs a reroll.

This would also really benefit from some before-and-after screenshots in the IS. It would be a lot easier to get a usability review if we make it easy on the reviewers. Tagging for an issue summary update to get some screenshots in the IS.

Re-rolled #72

I fixed the deprecation errors on patch #80 and updated the IS with the screenshots of the site with/without my patch.

I'm gonna check why there are still 2 errors left on the tests.

I tried but, couldn't figure out why the fails on the tests above...

They were random errors, sending back to needs review.

+1 for the patch, #82 works. Thanks

Hi,
I'll review

#82 patch works just fine.

This still needs a usability review. I'm setting back to NR.

I added a comment to the upcoming Usability meeting in an attempt to get this on their radar. I'm not sure I'll be able to join this meeting, but anyone who really wants this to get to RTBC needs to communicate with the Usability team.

#3299065: Drupal Usability Meeting 2022-07-29

We reviewed this issue in the UX meeting on Sept. 2nd. The agreed upon solution was to hide the disabled radio options and replace them with these words above revert button:

"Layout Builder is enabled for this view mode, and some content items have their layouts customized. Before Layout Builder can be disabled, you must revert these overrides."

The revert button text should be changed to "Revert layout overrides"

Here's a screenshot of the suggested solution:

The attendees on the call were AaronMcHale, benjifisher, rkoller, shaal, simohell, and myself. The recording is available here: https://youtu.be/iPWyziQHAFk

#3306259: Drupal Usability Meeting 2022-09-02

Something else that probably should be looked at.

The "Revert layout overrides" link, in Claro, uses a very big, very scary looking red button style, it draws all focus, even though it is not the primary action button on the page (Save is the primary button).

#button_type' => 'danger', is probably not appropriate, particularly as this link takes the user to a confirm form, there's nothing actually dangerous about this link. I would opt for using the normal button style, essentially what the "Manage layout" link is using.

I think this is probably a major bug.

Added patch against #99 in 10.1.x version

Ignore patch #99 above, additional work should be based on #82

@Deshna Chauhan There are several things incorrect with your patch in #99

1. A patch was not needed - the patch in #82 applies to 10.1.x just fine, we can see that from the test results
2. The version you supplied excludes several files, the fact that it is a a 6KB reroll of a 19KB patch it's removing other people's work and effectively breaking the patch. This is very disruptive as people may assume the most recent patch is the one to build from.

I've seen similar patch problems with a few contributors from Dotsquares. If anyone from that company would like guidance on finding issues that legitimately need rerolls and how to do those rerolls properly, have them find me on Drupal Slack and I'm happy to help.

The next step in this issue is to address @DyanneNova's review from #94 (starting from #82 as @bnjmnm indicates), and possibly #96 as well. That said, I am not sure I agree with this statement:

#button_type' => 'danger', is probably not appropriate, particularly as this link takes the user to a confirm form, there's nothing actually dangerous about this link. I would opt for using the normal button style, essentially what the "Manage layout" link is using.

It is deleting data -- possibly a lot of data -- so it should probably be styled however we elsewhere style buttons that delete lots of data. Maybe it's the confirm form button that needs that styling?

I've updated the IS and the Remaining Tasks based on previous comments, starting with the UX review in #94. Regarding Remaining Task #3

• Comment #96 - as the user is taken to a confirm form, there's nothing actually dangerous about this link.
• Comment #101 - the end result is that the user will potentially be deleting a lot of data. Should we instead style the confirm form button?

I didn't find instances of "deleting lots of data" in core, which is probably by design since it's dangerous. The only Danger Button that I saw was in the views_ui when deleting a field. (There is no confirmation message for this action, but it's not really "dangerous" because you're deleting config, which means this could potentially be a separate ticket to remove the Danger Button here.) I found several examples of core using action-link--danger when there is a Delete link that's followed by a confirmation page. In Claro, it looks like a red link with a trash can icon [screenshot], and the confirmation page uses the `button--primary` (blue button) for the Delete button. But, again, the examples I found are NOT destructive data deletions.

Based on my understanding of the User Interface Standards for buttons, the Big Red Button on the Manage Display page is not appropriate, as it is not part of a "set of other buttons." However, the Confirm button on the confirmation page could use the Danger Button as it is a destructive alternative.

Use danger only for a destructive button if it appears as an alternative option within a set of other buttons.

All that being said, this question may be a unique case without a precedent (I didn't find one in core, but maybe someone else has an example), and it's possible that the Standards need to be updated to cover this specific situation.

And not to completely derail this conversation or push anyone's buttons (haha), but it may be worth noting that there are arguments, e.g., this 7 year old debate, for not using Big Red Buttons at all.

Updates based on UX Review.

Assuming that we're going with the suggested solution in #94, here's a patch based on #82 that replaces the disabled fields with markup and updates the button text.

Updated screenshots (in Claro theme) to reflect comments from UX review (comment #94).

Thanks for working on this. The patch applies (with a few offsets) against the latest dev-release, and I can revert a Layout, which was locked.

The layout of the button is slightly broken, though: The "Revert layout overrides" is shown at the end of the text, to the right, and not below.

I've just used a patch from @hswong3i's #116 merge request on D10.4.1 and it works fine for me. Is it true that the only thing holding this up is the button?