Implement add button for top level item (section)

@hooroomoo raised terminology/functionality questions in a meeting yesterday wrt "sections". I continued that over at #3455036-13: [research] Clarify "components" vs "elements" vs "patterns" → , to make sure we formally define these so that it'll be clearer for the next contributor.

This work is built on top of 📌 Implement component states Fixed so those commits are in the MR as well. The logic and architecture is basically done other than some details that need to be worked out. The logic is if a node is on the root level, than the Add Section button renders. If the node has a parent, then the Add component button renders.

- All primary menu related actions now live in its own slice called primaryMenuSlice.ts
- Introduces a new ClickToInsertState in the slice.
- The logic to handle where to insert a node based on if it is a component vs. section lives in clickToInsertHandler in List.tsx
- AddButton component dispatches necessary info to the primaryMenuSlice which is used by the clickToInsertHandler.
- Replaced Submenu with another Menubar.Root so we can control opening the inner menu in redux.

Still left to do:
- Decide on behavior after user has inserts a new node. Should the user be able to insert multiple nodes consecutively or should the menu close?
- Fix styling regression on second level menu where hovered
- General styling for the new 'Add Component/Section' button.
- Tests
- Update issue summary to not deal with slots

ADD SECTION

ADD COMPONENT

Thanks for those GIFs in #7 🤩 Adding them to the issue summary to encourage somebody else to continue where @hooroomoo left things (thanks for the clear write-up about the current state in #6 too! 👏), because @hooroomoo is out for a few days 🏝️

Assigning to @Utkarsh_33! 😊

Welcome, @Utkarsh_33! 😄🥳

In the latest commit i was able to get the following things done that were remaining from #6 📌 Implement add button for top level item (section) Fixed :-

1)Close the sidebar menu once the user selects section/component.
2)Fixed styling regression on second level menu where hovered.
3)Centred the Add section/component button.

Still left to do::-
- Check it works with undo/redo
- Tests
- Update issue summary to not deal with slots.

Hmm, this doesn't look same as the design?

Now the button is more inline with the design.Attaching the screenshots for the same.

Final state:-

The "Add section" button should be only active when the component is active / selected, meaning that it should not be visible when hovering over a component that is not currently active / selected. Currently the button appears in the hover state, even when the component is not active.

@jessebaker: Is it okay for @Utkarsh_33 to continue here?

@Wim Leers

Yes this can be continued. I recommend that when @hooroomoo is back tomorrow that they and @Utkarsh_33 sync to ensure there is a clear decision as to who continues this work to push it over the finish line.

Once #3458369 📌 DDEV support for Cypress tests Fixed is complete and tests are easier to write I think some tests to ensure the following should be implemented

1. Ensure that after selected a component at the root level, a button is shown that says "Add section". Clicking that button should open the menu to the Section Templates part of the left menu.
2. Ensure that after selected a component below the root level (e.g. a component inside a slot), a button is shown that says "Add component". Clicking that button should open the menu to the Components part of the left menu.

Thanks, those are very clear instructions! 👍😊

This should be rebased again now that 📌 Implement component states Fixed which this MR was originally built on top of is in HEAD :)

@Utkarsh_33 FYI: see what @hooroomoo wrote in #21, which I explained in some more detail at https://git.drupalcode.org/project/experience_builder/-/merge_requests/9... 😊

Clearly, @Utkarsh_33 has already started writing tests prior to 📌 DDEV support for Cypress tests Fixed being ready, so removing the prefix 🚀

I still think that one #3458369: DDEV support for Cypress tests is in then it would be easier for me to resolve the test failures.

Fixed AddSection test!

I tried reverting the changes to a11y.cy.js and xb-general.cy.js, because I was 99% convinced that those changes were unnecessary, and were causing the tests to fail.

I was wrong! 😅

I will fix those.On my way!

Addressed major bunch of feedbacks.Just a small question left then it's again up for a review.

The issue summary mentions both "add section" and "add component" (for slots). I do not see the latter here - that should either be addressed int he MR or given its own issue (the additional issue might be more manageable IMO)

#36

The issue summary mentions both "add section" and "add component" (for slots). I do not see the latter here - that should either be addressed int he MR or given its own issue (the additional issue might be more manageable IMO)

Add component button is supported on the frontend side. But currently we don't have testable nested component/slots yet so there's no way to test it besides with the hard-coded layout-default.json file.

Add component button is supported on the frontend side. But currently we don't have testable nested component/slots yet so there's no way to test it besides with the hard-coded layout-default.json file.

It should be possible to use an intercept in Cypress to have a test that uses the layout-default.json file instead of the backend data. If that isn't possible, explaining why in a comment will be helpful.

It should be possible to use an intercept in Cypress to have a test that uses the layout-default.json file instead of the backend data. If that isn't possible, explaining why in a comment will be helpful.

It was possible to have an e2e test to use our mock json files. However, our front-end code throws many console errors when dealing with nested components (which makes sense because we haven't had to support it yet). Below is a photo of an artifact from the test run. I looked into it and the app gets confused of what is the currently selected component when it hovers over a nested component so sometimes the selectedElement is null which results in an error being thrown. The value eventually gets set so it passes locally and when doing it manually but in the CI, it decides to crap out whenever there is an error.

Since supporting nested components is out of scope for this issue, I updated the IS and added a todo in add-section.cy to add a test when we work on supporting nested components.

to add a test when we work on supporting nested components.

Ohh! I thought the UI/front end was ahead of the back end, in that it did support nested components already, I'd swear I'd seen that in a brief demo you did, @hooroomoo? 😅 Or perhaps that just happened to be a subset of the UI functionality for which enough pieces were in place already, i.e. maybe it's just the component hovering/selecting that does not yet support nested components?

our front-end code throws many console errors when dealing with nested components (which makes sense because we haven't had to support it yet). I looked into it and the app gets confused of what is the currently selected component when it hovers over a nested component so sometimes the selectedElement is null

👆 AFAICT this means we need a new issue for the front end, because this means the acceptance criteria for 📌 Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed are now vaguer: I thought the UI was sufficiently far ahead to be able to observe the entire UX, but knowing this, I think having that only preview a component tree is sufficient, @tedbow should not expect to be able to selector or edit it.

Would a new issue titled Add support for selecting a nested component be accurate?

@jessebaker is back from vacation today, I'll meet with him in a bit, so I'll ask him to start here :)

#42:

I almost want to wait until the backend can support/provide the frontend with a nested component before fixing the console issues from hovering over a nested component so we don't make changes based off of hard-coded JSON file where the structure might become outdated

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