Cancel button on the discard changes in the layout builder confirmation step should take you back to the layout builder

aaron.ferris → made their first commit to this issue’s fork.

Currently both actions (Confirm/cancel) result in $this->sectionStorage->getRedirectUrl()

Pushed a commit to the fork, which:

1. On discard confirm - redirects to node/x (with changes discarded) - same as the current behaviour.
2. On discard cancel - redirects to node/x/layout (with changes still in tact)

aaron.ferris → changed the visibility of the branch 3427398-the-functionality-of to hidden.

aaron.ferris → changed the visibility of the branch 3427398-the-functionality-of to active.

aaron.ferris → changed the visibility of the branch 3427398-the-functionality-of to hidden.

Awesome, thanks for the MR! I've manually tested and confirm that the MR provides the functionality I've suggested in the issue summary and that it feels like the right choice when actually playing around with it on an install of 11.x-dev. The only question would it be necessary to provide test(s) for this MR as well?

Manually Tested the MR and confirm that it resolved the issues regarding the Cancel and Confirm button functionality so RTBC+ From my side.

Changing the issue's status to RTBC, so that other people can also review and provide their suggestions.
As mentioned by @rkoller not very sure about the test coverage.

I checked the test coverage, unless im missing something (could well be) there is no coverage for the actual destination of these buttons.

There's this:

    // Make and then discard changes.
    $this->assertModifiedLayout(static::FIELD_UI_PREFIX . '/display/default/layout');
    $page->pressButton('Discard changes');
    $page->pressButton('Confirm');
    $assert_session->pageTextNotContains('You have unsaved changes.');

Also some bits and pieces around Discard->confirm, but not cancel.

Setting back to NW for test coverage.

If there is no coverage for the cancel button at the moment, IMO we need a test in LayoutBuilderUiTest that does the following:

1. Go to the default layout
2. Update the default layout, add a section & a block
3. Press the discard button
4. Press the cancel button
5. Assert the newly added section & block are still there
6. Assert the address is the layout builder edit screen

Ive pushed an initial go at the test for this, it follows a similar approach to testUnsavedChangesMessage

Will this suffice? Suggestions welcome.

May need a rebase.

Thanks @smustgrave - committed your suggestion and rebased. Back for review.

Hi, Can someone please add the propr screenshots of this issue? so that i can be able to reproduce it properly.
Followed the given steps but changes are already looking fine to me as expected.
Thanks.

Hello @DishaKatariya

Ive added some screenshots that will hopefully help

Thanks
Aaron

I added a couple of thoughts to the MR. Nothing I feel super strongly about so leaving the status the same

Hi, I've verified and tested MR! 7042 and applied patch successfully on 11.x Version the changes are working as expected.

Following steps I followed:
1. Open local and Go to /admin/structure/types/manage/article/display/default
2. Activate Layout Builder
3. Change the order of the default blocks to Tags, Body, Image, Comments
4. Click the Discard changes button
5. Click confirm
6. Reapply the changes to the block order for the layout
7. Click the Discard changes button again
8. Click the Cancel button
9. Observed the changes here

Testing Result:
After changing the orders of the default blocks and click on Discard changes button > Clicked on Cancel button and user was redirected back to the page of the layout so the user is able to continue with the already made changes. Hence the changes are working as expected.

Attaching the videos for reference.

Hence moving to RTBC!
Thanks

@sdhruvi5142, thank you for the manual testing.

There are unresolved threads in the MR about the testing. This also should have a title that reflects what is being fixed.

I've incorporated the new test into an existing test around the same functionality - which was missing a test of the cancel functionality.

Thanks @alexpott

Couldn't run test-only feature due to gitlab roles so ran locally

Behat\Mink\Exception\ExpectationException : Current page is "/admin/structure/types/manage/bundle_with_section_field/display/default", but "/admin/structure/types/manage/bundle_with_section_field/display/default/layout" expected.

Can confirm applying the MR that discarding changes takes me back to the layout builder.

Believe this change is good to go, title also reflects what is happening now.

Committed and pushed 8c50a65a7d to 11.x and 828a3c56df to 10.3.x and eb9a102582 to 10.2.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.