feat: add auto-scroll to canvas elements when selected from panel #3344
Conversation
packages/toolpad-studio/src/toolpad/AppEditor/PageEditor/RenderPanel/RenderOverlay.tsx
Outdated
Show resolved
Hide resolved
| if (!nodeId) { | ||
| return; | ||
| } | ||
| invariant(appRootRef.current, 'App root not found'); |
There was a problem hiding this comment.
Maybe you can also just check if appRootRef.current is truthy instead of using invariant and just return if not?
There was a problem hiding this comment.
Just copy pasted that line for the one of the command written above, should I remove the invariant?
There was a problem hiding this comment.
Yeah, not a big deal but I think it makes more sense to just not scroll if this ref isn't there yet.
…pad into feat/auto-scroll-canvas
Do you see any error or if possible can you also try on some other browser? I'm using brave which basically chrome and it's working fine for me. |
No errors at all, I tried to debug but couldn't find what's wrong so far... doesn't seem to work for me in other browsers either. |
|
Doesn't seem to work for me either in Brave: Screen.Recording.2024-04-03.at.6.31.45.PM.mov |
Ah, I just realized that I had the |
| @@ -152,6 +151,15 @@ export default function AppCanvas({ basename, state: initialState }: AppCanvasPr | |||
| return { nodes }; | |||
| }); | |||
|
|
|||
| setCommandHandler(bridge.canvasCommands, 'scrollComponent', (nodeId) => { | |||
| if (!nodeId) { | |||
There was a problem hiding this comment.
Just another thing I noticed, not very important either: when you call scrollComponent from the RenderOverlay component you already check if selectedNode is defined. So I guess that we don't need to check if nodeId is set here?
|
Thank you for looking into this feature. Works well (on classic canvas mode). The inline canvas will be the default soon, we should make sure the scroll keeps working under that mode. I've also pushed an integration test for scroll behavior on your PR. For context: the inline canvas mode removes the iframe from the Toolpad editor in which the Toolpad app runs and runs the Toolpad editor + Toolpad application in the same React tree. The ultimate goal for this is to eventually be able to run Toolpad as a React component inside of Next.js (or any other React runtime). You can try out this mode out by adding a EXPERIMENTAL_INLINE_CANVAS=1Currently the code for this contains some complexity to support both modes but we should be able to simplify as soon as the iframe is discontinued. Apologies for the inconvenience. |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
|
Hi, guys. Sorry was away for past few days. I'm good with the changes for the new canvas made the changes in both old canvas and new canvas container can you guys take a look.
|
|
great work, thank you! I've resetted the changes to the test fixture as it seems to be unnecessary |
Closes #3338
Adds auto-scroll to page canvas when element selected from the component panel.
Kapture.2024-03-29.at.00.37.24.mp4