feature: add component toolbar portal target #360
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@4leite is attempting to deploy a commit to the Measured Team on Vercel. A member of the Team first needs to authorize it. |
|
adds support for #150 |
| width: auto; | ||
| top: -48px; | ||
| top: -37px; |
There was a problem hiding this comment.
I don't think moving the toolbar here is the right solution, as leaving space between the toolbar and the border is a deliberate design choice that gives. I agree the flashing is annoying, but I think the better solution would be to create an invisible spacer between them that prevents the hover from bubbling up.
There was a problem hiding this comment.
Yup happy to implement that approach instead.
| @@ -97,6 +97,7 @@ export const DraggableComponent = ({ | |||
| {label && ( | |||
| <div className={getClassName("actionsLabel")}>{label}</div> | |||
| )} | |||
| <div id={`DraggableComponentToolbarPortal-${id}`} /> | |||
There was a problem hiding this comment.
What's the purpose of this portal? To inject new actions into the toolbar?
If so, I think a first party API might be a cleaner way of doing this instead of asking the user to inject via a portal. Something like <Puck toolbarActions={<div />} />
There was a problem hiding this comment.
I want to be able to inject from the inline editor. eg https://inline-rich-text.vercel.app/edit (click on the heading).
In the above example they are at the end of toolbar, but I think the middle is better (that was just the easiest for me to target with getElement / css Query hack).
These are dynamically changing based on lexical plugins and contextual for each draggable component.
An alternative would be to set them via setUI, but I do think the portal option is much cleaner for my particular use case.
There was a problem hiding this comment.
I think this API needs a bit more thought before we move forwards:
- I don't think we should be using portals for some elements of custom UI, and props for others.
- Users may wish to remove existing overlay controls (possibly a separate, but related API)
I'm now tracking this here: #377
There was a problem hiding this comment.
I would still like the (amended) UI fix for the dropdown, which I can put as a separate PR.
#377 is good, but... I would still at the least like a stable data-* attribute I can target in the mean time, even if it's an experimental, undocumented solution.
Note: This also adjusts the position of the toolbar, attaching it to the top border of the component to prevent the component above 'flashing' on hover. This becomes quite intrusive when interacting with the toolbar regularly.