Problem/Motivation

As soon as XB loads, the undo button is available because we set the initial state of the layout/model on load.

Steps to reproduce

Pressing undo immediately after loading /xb resets the preview/tree to a blank state.

I think the solution is going to be something like using the "Filter" option to prevent an undo state being created when we call dispatch(setLayoutModel({ layout: fetchedLayout.layout, model: fetchedLayout.model })) in Layout.tsx

CommentFileSizeAuthor
#27 2024-07-03 14.36.32.gif312.42 KBjessebaker
#9 Screenshot 2024-06-07 at 12.44.58 PM.png241.82 KBwim leers
2024-06-06 09.24.32.gif743.33 KBjessebaker

Issue fork experience_builder-3452895

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jessebaker created an issue. See original summary.

jessebaker’s picture

jessebaker commented
Title: undo/redo - user can undo the loading of the initial state » Undo/redo - user can undo the loading of the initial state

larowlan made their first commit to this issue’s fork.

larowlan opened merge request !41

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review

Added fix and tests on top of #3452781: Add tests for undo/redo

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Parent issue: » #3450592: [META] Front-end Kanban issue tracker

@larowlan Does that mean you think we should not fix this bug until #3452781: Add tests for undo/redo is done, i.e. to use this issue to establish the first regression test?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

No not at all
We can cherry pick the fix if need be

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: hooroomoo » jessebaker
Status: Needs review » Reviewed & tested by the community

@larowlan 👍

Oh WAIT THIS IS ALREADY ADDING A UNIT TEST! 🤩

I had missed that work already happened on the MR, and that @larowlan already wrote the test and added it to CI 😄

👏

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +gitlabci-contrib
StatusFileSize
new Screenshot 2024-06-07 at 12.44.58 PM.png241.82 KB

I made the new vitest CI job run immediately, which reduces the feedback loop for work on the UI to a minimum 🥳

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

@jessebaker Do we want vitest from here, or Cypress unit tests from #3452781: Add tests for undo/redo?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs review

Per #10.

jessebaker opened merge request !47

  • jessebaker committed ba07dcc5 on 0.x 
    Issue #3452895 by jessebaker, Wim Leers, larowlan, hooroomoo: Undo/redo...

jessebaker credited hooroomoo.

jessebaker’s picture

jessebaker commented
Status: Needs review » Fixed
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Fixed » Needs work

Unfortunately the added Cypress test doesn't seem to be running, there's no mention of layout slice in the CI output that I can see

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Indeed. I spotted the same on Friday, and @jessebaker & @bnjmnm told me they were on it. I'm sure they'll share some news later today.

Wim Leers closed merge request !41

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: jessebaker » bnjmnm

@bnjmnm is working on this 👍

bnjmnm made their first commit to this issue’s fork.

bnjmnm opened merge request !51

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review

cypress-component-and-unit-tests-should-run branch is here to address the test setup

  • jessebaker committed 0338812d on 0.x authored by bnjmnm 
    Issue #3452895 by bnjmnm, jessebaker, Wim Leers, larowlan, hooroomoo:...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: bnjmnm » jessebaker
Status: Needs review » Reviewed & tested by the community

Aaaand now Cypress unit + functional tests are running on GitLab CI: https://git.drupalcode.org/project/experience_builder/-/jobs/1900255 — thanks to @jessebaker for reviewing + merging, but HUGE thanks to @bnjmnm for getting it to this point!

@jessebaker:

Approving because I'm keen to see this merged - but I've left a couple of comments to keep in mind when take the testing further

Could you please create one or more new issues for your remarks, to ensure we do not forget about them? 🙏

jessebaker’s picture

jessebaker commented
Status: Reviewed & tested by the community » Fixed
jessebaker’s picture

jessebaker commented
Status: Fixed » Reviewed & tested by the community
jessebaker’s picture

jessebaker commented
Assigned: jessebaker » Unassigned
Issue summary: View changes
StatusFileSize
new 2024-07-03 14.36.32.gif312.42 KB

I am still seeing the issue on loading the XB.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

@jessebaker: please create a new issue, otherwise this issue keeps growing in length (number of comments) and MRs (already two merged over the course of multiple weeks) and hence keeps growing in confusion.

So:

  1. one new issue for #27
  2. one new issue for " but I've left a couple of comments to keep in mind when take the testing further"
jessebaker’s picture

jessebaker commented
Related issues: +#3459333: Undo/redo - user can undo the loading of the initial state (regression), +#3456020: Harden Cypress E2E test against environment variability + document Cypress test best practices

please create a new issue

Done:

1. https://www.drupal.org/project/experience_builder/issues/3459333
2. https://www.drupal.org/project/experience_builder/issues/3456020

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues:

Thanks!

Status: Fixed » Closed (fixed)

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