feat(core): support insert menu options in array item context menus #6921
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
No changes to documentation |
|
Component Testing Report Updated Jun 18, 2024 8:45 AM (UTC)
|
christianhg
force-pushed
the
feat-insert-menu-options-in-context-menu-take-2
branch
from
7c4d610
to
afdaeac
Compare
christianhg
force-pushed
the
feat-insert-menu-options-in-context-menu-take-2
branch
from
afdaeac
to
87cf8ea
Compare
christianhg
force-pushed
the
feat-insert-menu-options-in-context-menu-take-2
branch
from
87cf8ea
to
a401544
Compare
christianhg
force-pushed
the
feat-insert-menu-options-in-context-menu-take-2
branch
from
a574723
to
e8a7997
Compare
There was a problem hiding this comment.
The code looks great! I had one UX suggestion in there but besides that I couldn't find any other issues.
I would like to encourage adding more tests if you have the time. On the studio team, we're trying to stricter about quality and culturally we want everyone to include tests for new surfaces added in the stduio. this can be e2e or jest.
| }, | ||
| popoverProps: { | ||
| referenceElement: props.referenceElement, | ||
| placement: 'top-end', |
There was a problem hiding this comment.
do you think it would be better to have this be consistent with insertAfter? I personally think setting them both to prefer 'bottom-end' feels better.
This video is the current experience.
CleanShot.2024-06-17.at.14.17.15.mp4
There was a problem hiding this comment.
The decision to alter between top-end and bottom-end is primarily made by @mariuslundgard. The idea is that the placement gives a little more context in terms of where the item is inserted (above or below).
christianhg
force-pushed
the
feat-insert-menu-options-in-context-menu-take-2
branch
from
e8a7997
to
af0ba05
Compare
Thank you for the feedback, @ricokahler ! I agree that it's a good idea to add some test cases so I've now written some: af0ba05 I appreciate the UX suggestion, but I think I'll leave it as it is unless @mariuslundgard says otherwise. |
Description
Building on top of #6853, this PR adds the insert menu options to the context menu of each item in an array field. The old submenu in the context menu has been replaced with a popover, allowing for a fuller insert menu if that is configured.
Works for reference items as well:
Arrays of primitive values still use the old context menu sub menu:
What to review
Review that you can still insert items as before and that the new insert menu options optionally can be configured and thereby will take effect in the context menu again. You can use the
pnpm dev:page-builder-studioto play around with the insert menu options if you like.Testing
Notes for release