Fix and improve resizing in editor (incl. Add Spacer component) #2818

apedroferreira · 2023-10-20T18:14:37Z

I've tried to bring back this old PR so we can merge it instead of just closing, it's a bit large though... The Spacer component could be separated.

Improves resizing and allows for more control over spacing in the editor overall:

Some improvements to the editor such as getting rid of single columns in a page (pages are already columns), and spreading columns in pages if a column moves to a page after a deleting/moving an element.
Component API changes: Instead of a resizableHeightProp, make it possible to set a defaultLayoutHeight and minimumLayoutHeight for each component. This means that all components can be resized using the same system, but for now to avoid bugs I've kept the old resizing behavior: only charts and data grid are resizable, and only from the bottom border. This way no new bugs are introduced until we find solutions for some issues in the UX, but this new system is already there.
Adds a new Spacer component that can be used just to create white space. Maybe it could be useful but we can remove it from this PR too.

Spacer component demo:

Screen.Recording.2024-02-23.at.16.48.11.mov

Column spread demo:

Screen.Recording.2024-02-23.at.16.48.55.mov

Improved resizing demo (all components would be resizable + could be resized from top border, but this feature shown here is commented out for now as it still has some poor UX to improve in some scenarios):

Screen.Recording.2024-02-23.at.16.50.40.mov

…ments

…explorer headers

packages/toolpad-app/src/canvas/index.tsx

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


apedroferreira · 2023-11-02T14:56:12Z

packages/toolpad-app/src/appDom/index.ts

@@ -816,6 +817,21 @@ export function moveNode<Parent extends AppDomNode, Child extends AppDomNode>(
  return setNodeParent(dom, node, parent.id, parentProp, parentIndex);
 }

+export function spreadNode<Child extends AppDomNode>(dom: AppDom, node: Child) {


If an action in the editor causes a column of elements to be inside the page root, for example, we can spread the contents of the column on the page root directly and get rid of the column element.

apedroferreira · 2023-11-02T14:57:10Z

packages/toolpad-app/src/server/localMode.ts

+  element: ElementType,
+  isPageChild = false,
+): ElementType | ElementType[] {
+  if (isPageChild && element.component === PAGE_COLUMN_COMPONENT_ID) {


Gets rid of page columns inside the page root too, I saw it in a few apps already and it's unnecessary / shouldn't ever happen.

apedroferreira · 2023-11-02T15:02:40Z

packages/toolpad-components/src/Chart.tsx

@@ -205,7 +204,8 @@ export default createBuiltin(Chart, {
  loadingProp: 'loading',
  loadingPropSource: ['data'],
  errorProp: 'error',
-  resizableHeightProp: 'height',
+  defaultLayoutHeight: 360,


New interface that should be simpler.
Any component is vertically resizable, but we can set a default and minimum height for specific components.

packages/toolpad-app/src/toolpad/AppEditor/PageEditor/RenderPanel/NodeHud.tsx

-  const hintPosition = rect.y > HUD_HEIGHT ? HINT_POSITION_TOP : HINT_POSITION_BOTTOM;
+  const { nodeId: pageNodeId, viewState } = usePageEditorState();
+
+  const { nodes: nodesInfo } = viewState;


This reverts commit 9e967de.

apedroferreira · 2024-02-21T17:43:33Z

packages/toolpad-app/src/runtime/ToolpadApp.tsx

@@ -1195,6 +1193,8 @@ function RenderedNodeContent({ node, childNodeGroups, Component }: RenderedNodeC
            display: 'flex',
            alignItems: boundLayoutProps.verticalAlign,
            justifyContent: boundLayoutProps.horizontalAlign,
+            height: node.layout?.height ?? componentConfig.defaultLayoutHeight,


Element heights are controlled in the boxes that wrap components instead of the components themselves.

Janpot · 2024-02-23T08:40:00Z

@apedroferreira Can you also add a demo video that demonstrates the new features?

apedroferreira · 2024-02-23T16:58:03Z

@apedroferreira Can you also add a demo video that demonstrates the new features?

I've added 3 demos to the description, hopefully it helps!

Janpot · 2024-02-23T17:25:10Z

Looking good:

I see one suspicious regression test: https://app.argos-ci.com/mui/mui-toolpad/builds/2595/78472283. Is this expected?
I'm not a huge fan of exposing the Spacer component. Ideally we'd treat it as an internal component that gets created automatically when manipulating the UI. But I guess we can start like this and remove it down the line?
Do Spacer components collapse? e.g. if you have "component - spacer - component - spacer - component" and you delete the middle component, do you end up with "component - spacer - spacer - component" or with "component - spacer - component"?

apedroferreira · 2024-02-23T17:33:20Z

I see one suspicious regression test: https://app.argos-ci.com/mui/mui-toolpad/builds/2595/78472283. Is this expected?

Yeah, it's because the default height of the data grid changed, I probably changed it to be a multiple of the resizing intervals.

I'm not a huge fan of exposing the Spacer component. Ideally we'd treat it as an internal component that gets created automatically when manipulating the UI. But I guess we can start like this and remove it down the line?

Yeah I just didn't want to delete it right away from this PR but we had discussed not having it and it just being created automatically... As we're not sure about it I don't mind removing it and seeing what we come up with later if we come back to this, I guess it's better than introducing something we might not plan to keep. I'll remove it from the PR if you don't disagree.

Do Spacer components collapse? e.g. if you have "component - spacer - component - spacer - component" and you delete the middle component, do you end up with "component - spacer - spacer - component" or with "component - spacer - component"?

Nope, not in this version of them. Again I guess that I can remove it for now anyway.

Janpot · 2024-02-23T18:04:07Z

I'll remove it from the PR if you don't disagree.

I'm fine with keeping it

Janpot

impressive

This reverts commit 6e1e06c.

apedroferreira added 11 commits September 4, 2023 22:06

Allow DataGrid height to be styled, fix HUD position in full-page ele…

1fc9961

…ments

Only use bottom inner hint position if defaulting to bottom position

87cbb61

Update NodeHud.tsx

e04aff2

Merge branch 'master' into fix-containers

340041f

Fix creating new page with blur and default name + scroll only below …

95d16f6

…explorer headers

Fix resizing inside columns

50b9c7d

Merge remote-tracking branch 'origin/fix-containers' into resizing-fixes

eb1e0b7

Merge remote-tracking branch 'upstream/master' into resizing-fixes

cf7f783

Fix resizing, add Spacer component (WIP)

97ec428

Merge remote-tracking branch 'upstream/master' into resizing-fixes

4fb61b3

Vertical resizing with spacer (ongoing)

20a5764

apedroferreira added bug 🐛 Something doesn't work feature: App Editor labels Oct 20, 2023

apedroferreira self-assigned this Oct 20, 2023

apedroferreira added 3 commits October 20, 2023 19:22

Fix duplication in page root

62e3ce1

Merge remote-tracking branch 'upstream/master' into resizing-fixes

4019fcb

Fix more bugs, new layout height system

fbfca0b

apedroferreira changed the title ~~Fix and improve resizing in editor (WIP)~~ Fix and improve resizing in editor Nov 1, 2023

apedroferreira added 2 commits November 1, 2023 19:55

Add snapshot tests for vertical resizing of elements

6f0aaec

Merge remote-tracking branch 'upstream/master' into resizing-fixes

1a40eaa

apedroferreira marked this pull request as ready for review November 1, 2023 19:58

apedroferreira marked this pull request as draft November 1, 2023 19:59

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 2, 2023

apedroferreira commented Nov 2, 2023

View reviewed changes

packages/toolpad-app/src/canvas/index.tsx Outdated

handleScreenUpdate();

}, []);

React.useEffect(() => {

This comment was marked as outdated.

Sign in to view

apedroferreira commented Nov 2, 2023

View reviewed changes

apedroferreira force-pushed the resizing-fixes branch from 1f0c05f to 1a40eaa Compare November 2, 2023 15:12

apedroferreira commented Nov 2, 2023

View reviewed changes

Improve Spacer component height

007c311

apedroferreira added 3 commits February 21, 2024 15:46

Revert "Nevermind UX is bad anyway in some cases"

8608ab4

This reverts commit 9e967de.

Fix spread method

0efe12b

Re-add todos, this would take too much time

64ce555

apedroferreira commented Feb 21, 2024

View reviewed changes

apedroferreira added 5 commits February 21, 2024 17:50

Line spacing

f867615

More tests behaving differently for no reason

11caaa3

Still don't know what's wrong

01f7c34

Fix tests not passing if component catalog needs to scroll

16db5e6

I don't think this condition is ever true

f05c44b

apedroferreira requested a review from a team February 22, 2024 17:50

apedroferreira marked this pull request as ready for review February 22, 2024 17:50

apedroferreira added 2 commits February 22, 2024 18:09

Fix fixtures

ffcf428

Remove test.only

1ae8a85

Make Spacer height resizable

1dba2a3

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 23, 2024

apedroferreira added 2 commits February 23, 2024 17:43

Remove Spacer component

6e1e06c

Merge remote-tracking branch 'upstream/master' into resizing-fixes

5fcc01c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 23, 2024

Unused imports

19f3ef5

Janpot approved these changes Feb 23, 2024

View reviewed changes

apedroferreira added 2 commits February 23, 2024 18:05

Revert "Remove Spacer component"

7b9c763

This reverts commit 6e1e06c.

Readjust

bc14881

apedroferreira changed the title ~~Fix and improve resizing in editor~~ Fix and improve resizing in editor (incl. Add Spacer component) Feb 23, 2024

apedroferreira merged commit 4870ed8 into mui:master Feb 23, 2024
12 checks passed

apedroferreira deleted the resizing-fixes branch February 23, 2024 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix and improve resizing in editor (incl. Add Spacer component) #2818

Fix and improve resizing in editor (incl. Add Spacer component) #2818

apedroferreira commented Oct 20, 2023 •

edited

Loading

This comment was marked as outdated.

apedroferreira Nov 2, 2023

apedroferreira Nov 2, 2023 •

edited

Loading

apedroferreira Nov 2, 2023

This comment was marked as outdated.

apedroferreira Feb 21, 2024 •

edited

Loading

Janpot commented Feb 23, 2024

apedroferreira commented Feb 23, 2024

Janpot commented Feb 23, 2024 •

edited

Loading

apedroferreira commented Feb 23, 2024

Janpot commented Feb 23, 2024

Janpot left a comment

Fix and improve resizing in editor (incl. Add Spacer component) #2818

Fix and improve resizing in editor (incl. Add Spacer component) #2818

Conversation

apedroferreira commented Oct 20, 2023 • edited Loading

This comment was marked as outdated.

apedroferreira Nov 2, 2023

Choose a reason for hiding this comment

apedroferreira Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

apedroferreira Nov 2, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

apedroferreira Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

Janpot commented Feb 23, 2024

apedroferreira commented Feb 23, 2024

Janpot commented Feb 23, 2024 • edited Loading

apedroferreira commented Feb 23, 2024

Janpot commented Feb 23, 2024

Janpot left a comment

Choose a reason for hiding this comment

apedroferreira commented Oct 20, 2023 •

edited

Loading

apedroferreira Nov 2, 2023 •

edited

Loading

apedroferreira Feb 21, 2024 •

edited

Loading

Janpot commented Feb 23, 2024 •

edited

Loading