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

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

Merged
merged 47 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1fc9961
Allow DataGrid height to be styled, fix HUD position in full-page ele…
apedroferreira Sep 4, 2023
87cbb61
Only use bottom inner hint position if defaulting to bottom position
apedroferreira Sep 4, 2023
e04aff2
Update NodeHud.tsx
apedroferreira Sep 4, 2023
340041f
Merge branch 'master' into fix-containers
apedroferreira Sep 14, 2023
95d16f6
Fix creating new page with blur and default name + scroll only below …
apedroferreira Oct 11, 2023
50b9c7d
Fix resizing inside columns
apedroferreira Oct 12, 2023
eb1e0b7
Merge remote-tracking branch 'origin/fix-containers' into resizing-fixes
apedroferreira Oct 12, 2023
cf7f783
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Oct 12, 2023
97ec428
Fix resizing, add Spacer component (WIP)
apedroferreira Oct 16, 2023
4fb61b3
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Oct 17, 2023
20a5764
Vertical resizing with spacer (ongoing)
apedroferreira Oct 20, 2023
62e3ce1
Fix duplication in page root
apedroferreira Oct 20, 2023
4019fcb
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Oct 20, 2023
fbfca0b
Fix more bugs, new layout height system
apedroferreira Nov 1, 2023
6f0aaec
Add snapshot tests for vertical resizing of elements
apedroferreira Nov 1, 2023
1a40eaa
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Nov 1, 2023
007c311
Improve Spacer component height
apedroferreira Nov 2, 2023
5e25447
Fix highlight in slots
apedroferreira Nov 2, 2023
fa07e97
Update schemas
apedroferreira Nov 2, 2023
d2c5a8b
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Nov 2, 2023
0471351
Fix Chart component height
apedroferreira Nov 2, 2023
5a40939
Update docs
apedroferreira Nov 2, 2023
44fd577
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Feb 19, 2024
9cbe298
Continue merge, seeing if I can still turn this PR into something useful
apedroferreira Feb 19, 2024
189a104
More merge fixes
apedroferreira Feb 19, 2024
0e06cc0
Make elements always fill column space
apedroferreira Feb 19, 2024
afb84ae
More adjustments
apedroferreira Feb 19, 2024
3980eae
Regenerate docs
apedroferreira Feb 19, 2024
af6a74f
Block invalid top resize
apedroferreira Feb 19, 2024
7523bb7
Disable resizing from top for now, it's still not good enough
apedroferreira Feb 19, 2024
9e967de
Nevermind UX is bad anyway in some cases
apedroferreira Feb 19, 2024
8608ab4
Revert "Nevermind UX is bad anyway in some cases"
apedroferreira Feb 21, 2024
0efe12b
Fix spread method
apedroferreira Feb 21, 2024
64ce555
Re-add todos, this would take too much time
apedroferreira Feb 21, 2024
f867615
Line spacing
apedroferreira Feb 21, 2024
11caaa3
More tests behaving differently for no reason
apedroferreira Feb 22, 2024
01f7c34
Still don't know what's wrong
apedroferreira Feb 22, 2024
16db5e6
Fix tests not passing if component catalog needs to scroll
apedroferreira Feb 22, 2024
f05c44b
I don't think this condition is ever true
apedroferreira Feb 22, 2024
ffcf428
Fix fixtures
apedroferreira Feb 22, 2024
1ae8a85
Remove test.only
apedroferreira Feb 22, 2024
1dba2a3
Make Spacer height resizable
apedroferreira Feb 23, 2024
6e1e06c
Remove Spacer component
apedroferreira Feb 23, 2024
5fcc01c
Merge remote-tracking branch 'upstream/master' into resizing-fixes
apedroferreira Feb 23, 2024
19f3ef5
Unused imports
apedroferreira Feb 23, 2024
7b9c763
Revert "Remove Spacer component"
apedroferreira Feb 23, 2024
bc14881
Readjust
apedroferreira Feb 23, 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
9 changes: 4 additions & 5 deletions docs/data/toolpad/reference/components/chart.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ A chart component.

## Properties

| Name | Type | Default | Description |
| :------------------------------------ | :------------------------------------ | :------------------------------------ | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| <span class="prop-name">data</span> | <span class="prop-type">array</span> | | The data to be displayed. |
| <span class="prop-name">height</span> | <span class="prop-type">number</span> | <span class="prop-default">300</span> | The height of the chart. |
| <span class="prop-name">sx</span> | <span class="prop-type">object</span> | | The [`sx` prop](https://mui.com/system/getting-started/the-sx-prop/) is used for defining custom styles that have access to the theme. All MUI System properties are available via the `sx` prop. In addition, the `sx` prop allows you to specify any other CSS rules you may need. |
| Name | Type | Default | Description |
| :---------------------------------- | :------------------------------------ | :------ | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| <span class="prop-name">data</span> | <span class="prop-type">array</span> | | The data to be displayed. |
| <span class="prop-name">sx</span> | <span class="prop-type">object</span> | | The [`sx` prop](https://mui.com/system/getting-started/the-sx-prop/) is used for defining custom styles that have access to the theme. All MUI System properties are available via the `sx` prop. In addition, the `sx` prop allows you to specify any other CSS rules you may need. |
1 change: 0 additions & 1 deletion docs/data/toolpad/reference/components/data-grid.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ The datagrid lets users display tabular data in a flexible grid.
| <span class="prop-name">rowIdField</span> | <span class="prop-type">string</span> | | Defines which column contains the [id](https://mui.com/x/react-data-grid/row-definition/#row-identifier) that uniquely identifies each row. |
| <span class="prop-name">selection</span> | <span class="prop-type">object</span> | <span class="prop-default">null</span> | The currently selected row. Or `null` in case no row has been selected. |
| <span class="prop-name">density</span> | <span class="prop-type">string</span> | <span class="prop-default">"compact"</span> | The [density](https://mui.com/x/react-data-grid/accessibility/#density-prop) of the rows. Possible values are `compact`, `standard`, or `comfortable`. |
| <span class="prop-name">height</span> | <span class="prop-type">number</span> | <span class="prop-default">350</span> | The height of the data grid. |
| <span class="prop-name">loading</span> | <span class="prop-type">boolean</span> | | Displays a loading animation indicating the data grid isn't ready to present data yet. |
| <span class="prop-name">hideToolbar</span> | <span class="prop-type">boolean</span> | | Hide the toolbar area that contains the data grid user controls. |
| <span class="prop-name">sx</span> | <span class="prop-type">object</span> | | The [`sx` prop](https://mui.com/system/getting-started/the-sx-prop/) is used for defining custom styles that have access to the theme. All MUI System properties are available via the `sx` prop. In addition, the `sx` prop allows you to specify any other CSS rules you may need. |
4 changes: 4 additions & 0 deletions docs/schemas/v1/definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,10 @@
"columnSize": {
"type": "number",
"description": "The width this element takes up, expressed in terms of columns on the page."
},
"height": {
"type": "number",
"description": "The height this element takes up, in pixels."
}
},
"additionalProperties": false,
Expand Down
4 changes: 2 additions & 2 deletions packages/toolpad-app/src/runtime/ToolpadApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,8 @@ function getQueryConfigBindings({ enabled, refetchInterval }: appDom.QueryNode['
}

function isBindableProp(componentConfig: ComponentConfig<any>, propName: string) {
const isResizableHeightProp = propName === componentConfig.resizableHeightProp;
const argType = componentConfig.argTypes?.[propName];
return (
!isResizableHeightProp &&
argType?.control?.bindable !== false &&
argType?.type !== 'template' &&
argType?.type !== 'event'
Expand Down Expand Up @@ -1195,6 +1193,8 @@ function RenderedNodeContent({ node, childNodeGroups, Component }: RenderedNodeC
display: 'flex',
alignItems: boundLayoutProps.verticalAlign,
justifyContent: boundLayoutProps.horizontalAlign,
height: node.layout?.height ?? componentConfig.defaultLayoutHeight,
Copy link
Member Author

@apedroferreira apedroferreira Feb 21, 2024

Choose a reason for hiding this comment

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

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

minHeight: '100%',
}}
ref={nodeRef}
data-toolpad-node-id={nodeId}
Expand Down
14 changes: 11 additions & 3 deletions packages/toolpad-app/src/server/localMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ function expandFromDom<N extends appDom.AppDomNode>(
name: node.name,
layout: undefinedWhenEmpty({
columnSize: node.layout?.columnSize,
height: node.layout?.height,
horizontalAlign: stringOnly(node.layout?.horizontalAlign),
verticalAlign: stringOnly(node.layout?.verticalAlign),
}),
Expand Down Expand Up @@ -715,7 +716,14 @@ function mergePageIntoDom(dom: appDom.AppDom, pageName: string, pageFile: Page):
return dom;
}

function optimizePageElement(element: ElementType): ElementType {
function optimizePageElement(
element: ElementType,
isPageChild = false,
): ElementType | ElementType[] {
if (isPageChild && element.component === PAGE_COLUMN_COMPONENT_ID) {
Copy link
Member Author

@apedroferreira apedroferreira Nov 2, 2023

Choose a reason for hiding this comment

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

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.

return (element.children || []).flatMap((child) => optimizePageElement(child, true));
}

const isLayoutElement = (possibleLayoutElement: ElementType): boolean =>
possibleLayoutElement.component === PAGE_ROW_COMPONENT_ID ||
possibleLayoutElement.component === PAGE_COLUMN_COMPONENT_ID;
Expand All @@ -736,7 +744,7 @@ function optimizePageElement(element: ElementType): ElementType {

return {
...element,
children: element.children && element.children.map(optimizePageElement),
children: element.children && element.children.flatMap((child) => optimizePageElement(child)),
};
}

Expand All @@ -745,7 +753,7 @@ function optimizePage(page: Page): Page {
...page,
spec: {
...page.spec,
content: page.spec?.content?.map(optimizePageElement),
content: page.spec?.content?.flatMap((element) => optimizePageElement(element, true)),
},
};
}
Expand Down
1 change: 1 addition & 0 deletions packages/toolpad-app/src/server/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ const baseElementSchema = z.object({
.number()
.optional()
.describe('The width this element takes up, expressed in terms of columns on the page.'),
height: z.number().optional().describe('The height this element takes up, in pixels.'),
})
.optional()
.describe('Layout properties for this element.'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ function shouldRenderControl<P extends object>(
propTypeDef: ArgTypeDefinition<P>,
propName: keyof P,
props: P,
componentConfig: ComponentConfig<P>,
) {
if (propTypeDef.type === 'element' || propTypeDef.type === 'template') {
return (
Expand All @@ -60,10 +59,6 @@ function shouldRenderControl<P extends object>(
return propTypeDef.visible(props);
}

if (componentConfig.resizableHeightProp && propName === componentConfig.resizableHeightProp) {
return false;
}

return true;
}

Expand Down Expand Up @@ -141,7 +136,7 @@ function ComponentPropsEditor<P extends object>({
{category}:
</Typography>
{argTypeEntries.map(([propName, propTypeDef]) =>
propTypeDef && shouldRenderControl(propTypeDef, propName, props, componentConfig) ? (
propTypeDef && shouldRenderControl(propTypeDef, propName, props) ? (
<div key={propName} className={classes.control}>
<NodeAttributeEditor
node={node}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ export default function NodeDropArea({
const isPageNode = appDom.isPage(node);
const isPageChild = dropAreaNodeParent ? appDom.isPage(dropAreaNodeParent) : false;

const isPageRowNode = appDom.isElement(node) && isPageRow(node);

const isPageChildElement = isPageChild && appDom.isElement(node) && !isPageRow(node);
const isPageRowChild = dropAreaNodeParent
? appDom.isElement(dropAreaNodeParent) && isPageRow(dropAreaNodeParent)
Expand Down Expand Up @@ -242,7 +244,7 @@ export default function NodeDropArea({
}

// Is dragging over left, is page row and child of the page
if (dropAreaNodeParent && appDom.isElement(node) && isPageRow(node) && isPageChild) {
if (dropAreaNodeParent && isPageRowNode && isPageChild) {
return null;
}
}
Expand All @@ -252,21 +254,19 @@ export default function NodeDropArea({
if (
dropAreaNodeParent &&
dropAreaNodeParent.id === dragOverNodeId &&
pageAwareParentProp === dragOverSlotParentProp &&
!parentProp
(pageAwareParentProp === dragOverSlotParentProp || !parentProp)
) {
const parentLastChild =
pageAwareParentProp &&
(appDom.isPage(dropAreaNodeParent) || appDom.isElement(dropAreaNodeParent))
? appDom.getNodeLastChild(dom, dropAreaNodeParent, pageAwareParentProp)
appDom.isPage(dropAreaNodeParent) || appDom.isElement(dropAreaNodeParent)
? appDom.getNodeLastChild(dom, dropAreaNodeParent, pageAwareParentProp || 'children')
: null;

const isParentLastChild = parentLastChild ? node.id === parentLastChild.id : false;

const parentSlots = dropAreaNodeParentInfo?.slots || null;

const parentFlowDirection =
parentSlots && pageAwareParentProp && parentSlots[pageAwareParentProp]?.flowDirection;
parentSlots && parentSlots[pageAwareParentProp || 'children']?.flowDirection;

return parentFlowDirection && isParentLastChild
? getChildNodeHighlightedZone(parentFlowDirection)
Expand Down Expand Up @@ -298,18 +298,19 @@ export default function NodeDropArea({
? dragOverZone
: null;
}, [
dropAreaNodeParent,
dom,
dragOverZone,
availableDropZones,
isPageNode,
parentProp,
isEmptySlot,
dropAreaNodeParent,
dom,
isPageChild,
node,
dragOverNodeId,
dragOverSlotParentProp,
isPageRowChild,
isPageChild,
isPageRowNode,
dropAreaNodeParentInfo?.slots,
dropAreaNodeChildNodes,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
RECTANGLE_EDGE_BOTTOM,
RECTANGLE_EDGE_LEFT,
RECTANGLE_EDGE_RIGHT,
RECTANGLE_EDGE_TOP,
} from '../../../../utils/geometry';

const HINT_POSITION_TOP = 'top';
Expand Down Expand Up @@ -84,7 +85,10 @@ const SelectionHintWrapper = styled('div', {
zIndex: 1000,
...(hintPosition === HINT_POSITION_TOP
? { top: 0, transform: 'translate(0, -100%)' }
: { bottom: 0, transform: 'translate(0, 100%)' }),
: {
bottom: 0,
transform: 'translate(0, 100%)',
}),
},
}));

Expand Down Expand Up @@ -127,6 +131,15 @@ const DraggableEdge = styled('div', {
width: '100%',
};
}
if (edge === RECTANGLE_EDGE_TOP) {
dynamicStyles = {
cursor: 'ns-resize',
top: -10,
height: 22,
left: 0,
width: '100%',
};
}

return {
...dynamicStyles,
Expand Down Expand Up @@ -177,7 +190,10 @@ export default function NodeHud({
isHoverable = true,
isHovered = false,
}: NodeHudProps) {
const hintPosition = rect.y > HUD_HEIGHT ? HINT_POSITION_TOP : HINT_POSITION_BOTTOM;
let hintPosition: HintPosition = HINT_POSITION_BOTTOM;
if (rect.y > HUD_HEIGHT) {
hintPosition = HINT_POSITION_TOP;
}

const interactiveNodeClipPath = React.useMemo(
() =>
Expand Down
Loading
Loading