Overview

Clicking duplicate on the Two Column layout crashes the page.

Proposed resolution

I suspect the issue is that the slots and children in the layoutModel data are not being given new UUID's in layoutModelSlice.ts::duplicateNode - a recursive function to rename the UUID's in the layout and create new values with those UUID's in the model is required here

User interface changes

CommentFileSizeAuthor
#11 Screenshot 2024-09-09 at 11.08.01 AM.png229.62 KBwim leers
Screenshot 2024-09-06 at 12.46.56 PM.png736.4 KBfazilitehreem

Issue fork experience_builder-3472507

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

fazilitehreem created an issue. See original summary.

jessebaker’s picture

jessebaker commented
Issue summary: View changes
jessebaker’s picture

jessebaker commented
Priority: Normal » Major

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

utkarsh_33 opened merge request !266

utkarsh_33’s picture

utkarsh_33 commented
Assigned: Unassigned » jessebaker
Status: Active » Needs review
jessebaker’s picture

jessebaker commented
Assigned: jessebaker » utkarsh_33
Status: Needs review » Needs work

This is not passing the tests yet. Please ensure the tests pass before setting status to Needs review.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

I was just able to duplicate a two column component without issue.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

But after duplicating one with components in it, components in the slots are all gone.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Needs steps to reproduce

I cannot reproduce this either.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Clicking duplicate for two column layout on xb, crashes the site » `Duplicate` action does not generate new UUIDs for components in slots, resulting in duplicate UUIDs
Status: Needs work » Active
Issue tags: -Needs steps to reproduce
StatusFileSize
new Screenshot 2024-09-09 at 11.08.01 AM.png229.62 KB

Ah! I can, and the symptoms are actually super obvious:

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3472089: Improve/Refactor replaceUUIDsAndUpdateModel() and insertMultipleNodes()

This relates to #3472089: Improve/Refactor replaceUUIDsAndUpdateModel() and insertMultipleNodes(), in that we should have unit test coverage for this too.

utkarsh_33’s picture

utkarsh_33 commented
Assigned: utkarsh_33 » Unassigned

Currently not working on this.

utkarsh_33’s picture

utkarsh_33 commented
Assigned: Unassigned » utkarsh_33
utkarsh_33’s picture

utkarsh_33 commented
Assigned: utkarsh_33 » Unassigned
Status: Active » Needs review
utkarsh_33’s picture

utkarsh_33 commented
Assigned: Unassigned » jessebaker
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: jessebaker » Unassigned
Status: Needs review » Needs work
Issue tags: +Needs tests

Missing unit tests, as mentioned in #12.

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review

I have tried writing unit tests for the first time, so might not match the coding standards😅. Please review the changes.

balintbrews’s picture

balintbrews
he/him/his
Primary language Hungarian
Location Netherlands 🇪🇺
commented
Assigned: Unassigned » balintbrews

Assigning to myself for review.

balintbrews’s picture

balintbrews
he/him/his
Primary language Hungarian
Location Netherlands 🇪🇺
commented
Assigned: balintbrews » utkarsh_33
Status: Needs review » Needs work

Great job with the fix, @utkarsh_33! Duplicating components with slots filled with children works after your changes! 🚀 I left you a few comments on the code — fixing one regression and some clean-up remains to be done before we can merge.

utkarsh_33’s picture

utkarsh_33 commented
Assigned: utkarsh_33 » balintbrews
Status: Needs work » Needs review

I have updated the code according to requested changes.Please review them.Their might be some more feedbacks on 11bd33b4 change, but rest i think i have addressed most of the changes.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: balintbrews » utkarsh_33
Status: Needs review » Needs work

@balintbrews' feedback at https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... is not yet addressed.

utkarsh_33’s picture

utkarsh_33 commented
Assigned: utkarsh_33 » balintbrews
Status: Needs work » Needs review
balintbrews’s picture

balintbrews
he/him/his
Primary language Hungarian
Location Netherlands 🇪🇺
commented
Assigned: balintbrews » utkarsh_33
Status: Needs review » Needs work
utkarsh_33’s picture

utkarsh_33 commented
Assigned: utkarsh_33 » balintbrews
Status: Needs work » Needs review
balintbrews’s picture

balintbrews
he/him/his
Primary language Hungarian
Location Netherlands 🇪🇺
commented
Assigned: balintbrews » jessebaker

This looks good to me. Assigning to @jessebaker for a final round of review.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests
Parent issue: » #3450592: [META] Front-end Kanban issue tracker

jessebaker’s picture

jessebaker commented
Assigned: jessebaker » Unassigned
Status: Reviewed & tested by the community » Fixed

Merged!

Status: Fixed » Closed (fixed)

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