Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add auto-scroll to canvas elements when selected from panel #3344

Merged
merged 32 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
eaad9f3
Fix dedupe check
Janpot Mar 26, 2024
d36dad1
Merge remote-tracking branch 'upstream/master'
Janpot Mar 26, 2024
413c7bf
Merge remote-tracking branch 'upstream/master'
Janpot Mar 28, 2024
9268da2
Merge remote-tracking branch 'upstream/master'
Janpot Mar 28, 2024
a755a3d
Merge remote-tracking branch 'upstream/master'
Janpot Mar 28, 2024
173d3ea
feat: add auto-scroll to canvas elements when selected from panel
b4s36t4 Mar 29, 2024
d957560
revert: appRoot from returning
b4s36t4 Mar 29, 2024
58b7f82
Merge branch 'master' into feat/auto-scroll-canvas
b4s36t4 Mar 29, 2024
1e08581
chore: make instant scroll
b4s36t4 Mar 30, 2024
dbd2af8
Merge branch 'feat/auto-scroll-canvas' of github.com:b4s36t4/mui-tool…
b4s36t4 Mar 30, 2024
efbc979
Merge remote-tracking branch 'upstream/master'
Janpot Apr 4, 2024
80dd0d8
add test
Janpot Apr 4, 2024
9676ffd
Merge remote-tracking branch 'upstream/master'
Janpot Apr 5, 2024
703d127
Merge remote-tracking branch 'upstream/master'
Janpot Apr 7, 2024
1dc5912
Merge remote-tracking branch 'upstream/master'
Janpot Apr 8, 2024
34cc01f
Merge remote-tracking branch 'upstream/master'
Janpot Apr 9, 2024
3c1e6e8
Merge remote-tracking branch 'upstream/master'
Janpot Apr 11, 2024
c86f22b
Merge remote-tracking branch 'upstream/master'
Janpot Apr 15, 2024
d3399a1
Merge remote-tracking branch 'upstream/master'
Janpot Apr 15, 2024
9c901cd
Merge remote-tracking branch 'upstream/master'
Janpot Apr 17, 2024
701b6a7
Merge remote-tracking branch 'upstream/master'
Janpot Apr 17, 2024
718ae37
Merge remote-tracking branch 'upstream/master'
Janpot Apr 18, 2024
52af240
Merge remote-tracking branch 'upstream/master'
Janpot Apr 21, 2024
0d4ed0c
Merge remote-tracking branch 'upstream/master'
Janpot Apr 24, 2024
d943a64
Merge branch 'master' of github.com:mui/mui-toolpad into feat/auto-sc…
b4s36t4 Apr 26, 2024
be516bb
fix: support new canvas host scroll
b4s36t4 Apr 26, 2024
24e57a7
Merge remote-tracking branch 'upstream/master'
Janpot Apr 26, 2024
169871c
Merge remote-tracking branch 'upstream/master'
Janpot Apr 30, 2024
2ab76b8
Merge branch 'master' into feat/auto-scroll-canvas
b4s36t4 May 1, 2024
5f05493
Merge remote-tracking branch 'upstream/master'
Janpot May 2, 2024
93904ba
Merge branch 'master' into pr/3344
Janpot May 2, 2024
f9f69e8
Reset changes to test
Janpot May 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/toolpad-studio/src/canvas/ToolpadBridge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export interface ToolpadBridge {
canvasCommands: Commands<{
getViewCoordinates(clientX: number, clientY: number): { x: number; y: number } | null;
getPageViewState(): PageViewState;
scrollComponent(nodeId: string): void;
isReady(): boolean;
invalidateQueries(): void;
}>;
Expand All @@ -73,6 +74,9 @@ const bridge: ToolpadBridge | null = isRenderedInCanvas
getPageViewState: () => {
throw new Error('Not implemented');
},
scrollComponent: () => {
throw new Error('Not Implemented');
},
getViewCoordinates: () => {
throw new Error('Not implemented');
},
Expand Down
10 changes: 9 additions & 1 deletion packages/toolpad-studio/src/canvas/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ export default function AppCanvas({ basename, state }: AppCanvasProps) {

setCommandHandler(bridge.canvasCommands, 'getPageViewState', () => {
invariant(appRootRef.current, 'App root not found');

let nodes = viewState.current.nodes;

for (const [nodeId, nodeInfo] of Object.entries(nodes)) {
Expand All @@ -154,6 +153,15 @@ export default function AppCanvas({ basename, state }: AppCanvasProps) {
return { nodes };
});

setCommandHandler(bridge.canvasCommands, 'scrollComponent', (nodeId) => {
if (!nodeId) {
Copy link
Member

@apedroferreira apedroferreira Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

return;
}
invariant(appRootRef.current, 'App root not found');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can also just check if appRootRef.current is truthy instead of using invariant and just return if not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just copy pasted that line for the one of the command written above, should I remove the invariant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not a big deal but I think it makes more sense to just not scroll if this ref isn't there yet.

const canvasNode = appRootRef.current.querySelector(`[data-node-id='${nodeId}']`);
canvasNode?.scrollIntoView({ behavior: 'instant', block: 'end', inline: 'end' });
});

setCommandHandler(bridge.canvasCommands, 'getViewCoordinates', (clientX, clientY) => {
if (!appRootRef.current) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
AppHostProvider,
useAppHost,
queryClient,
NodeId,
} from '@toolpad/studio-runtime';
import createCache from '@emotion/cache';
import { CacheProvider } from '@emotion/react';
Expand Down Expand Up @@ -182,6 +183,13 @@ export default function EditorCanvasHost({
queryClient.invalidateQueries();
},
update: () => {},
scrollComponent: (nodeId: NodeId) => {
if (!appRoot) {
return;
}
const node = appRoot.querySelector(`[data-node-id='${nodeId}']`);
node?.scrollIntoView({ behavior: 'instant', block: 'end', inline: 'end' });
},
}),
} satisfies ToolpadBridge;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,17 @@ export default function RenderOverlay({ bridge }: RenderOverlayProps) {
};
}, [handleNodeDragEnd]);

const scrollSelectedNode = React.useCallback(() => {
if (!selectedNode) {
return;
}
bridge?.canvasCommands.scrollComponent(selectedNode.id as string);
}, [bridge?.canvasCommands, selectedNode]);

React.useEffect(() => {
scrollSelectedNode();
}, [scrollSelectedNode]);

const resizePreviewElementRef = React.useRef<HTMLDivElement | null>(null);

const overlayGridRef = React.useRef<OverlayGridHandle>({
Expand Down
3 changes: 3 additions & 0 deletions test/visual/components/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ test('rendering components in the app editor', async ({ page, argosScreenshot })

await clickCenter(page, image);
await argosScreenshot('with-selection');

await page.getByRole('treeitem', { name: 'typography1' }).click();
await argosScreenshot('with-selection-scroll');
});

test('building layouts', async ({ page, argosScreenshot }) => {
Expand Down
Loading