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: anvil layout and some cleanup #33442

Merged
merged 1 commit into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 0 additions & 36 deletions app/client/src/layoutSystems/anvil/integrations/layoutSelectors.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
import "./styles.css";
import { Flex } from "@design-system/widgets";
import type {
AlignSelf,
FlexDirection,
FlexProps,
FlexWrap,
JustifyContent,
Responsive,
SizingDimension,
SpacingDimension,
} from "@design-system/widgets";
import type { FlexProps } from "@design-system/widgets";
import React, { useMemo } from "react";
import type { CSSProperties, ReactNode } from "react";
import type {
OverflowValues,
PositionValues,
} from "layoutSystems/anvil/utils/types";
import type { PositionValues } from "layoutSystems/anvil/utils/types";
import { usePositionObserver } from "layoutSystems/common/utils/LayoutElementPositionsObserver/usePositionObserver";
import { getAnvilLayoutDOMId } from "layoutSystems/common/utils/LayoutElementPositionsObserver/utils";
import type { RenderMode } from "constants/WidgetConstants";
Expand All @@ -26,11 +14,7 @@ import {
getShouldHighLightCellSelector,
} from "layoutSystems/anvil/integrations/selectors";

export interface FlexLayoutProps
extends AlignSelf,
JustifyContent,
FlexDirection,
FlexWrap {
export interface FlexLayoutProps extends FlexProps {
canvasId: string;
children: ReactNode;
isContainer?: boolean;
Expand All @@ -39,35 +23,16 @@ export interface FlexLayoutProps
layoutIndex: number;
layoutType: LayoutComponentTypes;
parentDropTarget: string;
renderMode: RenderMode;

border?: string;
columnGap?: Responsive<SpacingDimension>;
flexBasis?: Responsive<SizingDimension>;
flexGrow?: Responsive<number>;
flexShrink?: Responsive<number>;
height?: Responsive<SizingDimension>;
maxHeight?: Responsive<SizingDimension>;
maxWidth?: Responsive<SizingDimension>;
minWidth?: Responsive<SizingDimension>;
minHeight?: Responsive<SizingDimension>;
overflowX?: OverflowValues;
overflowY?: OverflowValues;
position?: PositionValues;
gap?: Responsive<SpacingDimension>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All this we have in FlexProps type.

padding?: Responsive<SpacingDimension>;
width?: Responsive<SizingDimension>;
className?: string;
renderMode: RenderMode;
}

export const FlexLayout = React.memo((props: FlexLayoutProps) => {
const {
alignSelf,
border,
canvasId,
children,
className,
columnGap,
direction,
flexBasis,
flexGrow,
Expand All @@ -87,9 +52,9 @@ export const FlexLayout = React.memo((props: FlexLayoutProps) => {
padding,
parentDropTarget,
position,
renderMode,
width,
wrap,
...rest
} = props;
/** POSITIONS OBSERVER LOGIC */
// Create a ref so that this DOM node can be
Expand All @@ -111,7 +76,6 @@ export const FlexLayout = React.memo((props: FlexLayoutProps) => {
const flexProps: FlexProps = useMemo(() => {
return {
alignSelf: alignSelf || "flex-start",
columnGap: columnGap || "0px",
direction: direction || "column",
flexBasis: flexBasis || "auto",
flexGrow: flexGrow || 0,
Expand All @@ -130,7 +94,6 @@ export const FlexLayout = React.memo((props: FlexLayoutProps) => {
};
}, [
alignSelf,
columnGap,
direction,
flexBasis,
flexGrow,
Expand Down Expand Up @@ -160,7 +123,7 @@ export const FlexLayout = React.memo((props: FlexLayoutProps) => {
? { background: "var(--anvil-cell-highlight)" }
: {}),
};
}, [border, isDropTarget, position, renderMode, shouldHighlightCell]);
Copy link
Collaborator Author

@KelvinOm KelvinOm May 14, 2024

Choose a reason for hiding this comment

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

We don't use border and renderMode in this useMemo.

}, [isDropTarget, position, shouldHighlightCell]);

const _className = useMemo(() => {
return `${className ?? ""} layout-${layoutId} layout-index-${layoutIndex} ${
Expand All @@ -171,6 +134,7 @@ export const FlexLayout = React.memo((props: FlexLayoutProps) => {
return (
<Flex
{...flexProps}
{...rest}
className={_className}
id={getAnvilLayoutDOMId(canvasId, layoutId)}
ref={ref}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useMemo, useState } from "react";
import React, { useMemo } from "react";
import {
LayoutComponentTypes,
type LayoutComponentProps,
Expand All @@ -10,16 +10,8 @@ import {
} from "layoutSystems/anvil/utils/constants";
import { FlexLayerAlignment } from "layoutSystems/common/utils/constants";
import { renderWidgets } from "layoutSystems/anvil/utils/layouts/renderUtils";
import { isEditOnlyModeSelector } from "../../../../../selectors/editorSelectors";
import { FlexLayout, type FlexLayoutProps } from "../FlexLayout";
import { isFillWidgetPresentInList } from "layoutSystems/anvil/utils/layouts/widgetUtils";
import { getAnvilLayoutDOMId } from "layoutSystems/common/utils/LayoutElementPositionsObserver/utils";
import {
ALIGNMENT_WIDTH_THRESHOLD,
shouldOverrideAlignmentStyle,
} from "layoutSystems/anvil/integrations/layoutSelectors";
import { useSelector } from "react-redux";
import { RenderModes } from "constants/WidgetConstants";

/**
* If AlignedRow hasFillWidget:
Expand All @@ -31,72 +23,18 @@ import { RenderModes } from "constants/WidgetConstants";
* Each alignment has following characteristics:
* 1. Mobile viewport:
* - flex-wrap: wrap.
* - flex-basis: auto.
* ~ This ensures the alignment takes up as much space as needed by the children.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed flex-basis to 0% for mobile because I believe it's wrong. With auto the container with the component takes more space and the grid breaks.
May-14-2024 19-18-13

* ~ It can stretch to the full width of the viewport.
* ~ or collapse completely if there is no content.
*
* 2. Larger view ports:
* - flex-wrap: nowrap.
* - flex-basis: 0%.
* ~ This ensures that alignments share the total space equally, until possible.
* ~ Soon as the content in any alignment needs more space, it will wrap to the next line
* thanks to flex wrap in the parent layout.
*/
const AlignedWidgetRowComp = (props: LayoutComponentProps) => {
const { canvasId, layout, layoutId, renderMode } = props;
// Whether default alignment styles should be overridden, when renderMode = Canvas.
const shouldOverrideStyle: boolean = useSelector(
shouldOverrideAlignmentStyle(layoutId),
);

// check if layout renders a Fill widget.
const hasFillWidget: boolean = isFillWidgetPresentInList(
layout as WidgetLayoutProps[],
);

const [isAnyAlignmentOverflowing, setIsAnyAlignmentOverflowing] =
useState(false);

useEffect(() => {
// getBoundingClientRect is an expensive operation and should only be used when renderMode = Page,
// because layout positions are not available in that case.
if (hasFillWidget || renderMode !== RenderModes.PAGE) return;
const parentLayoutId = getAnvilLayoutDOMId(canvasId, layoutId);
KelvinOm marked this conversation as resolved.
Show resolved Hide resolved
const parentLayout = document.getElementById(parentLayoutId);
if (parentLayout) {
const parentLayoutWidth = parentLayout.getBoundingClientRect().width;

// Use requestAnimationFrame to ensure calculation is done after rendering
requestAnimationFrame(() => {
const isOverflowing = [
FlexLayerAlignment.Start,
FlexLayerAlignment.Center,
FlexLayerAlignment.End,
].some((each: FlexLayerAlignment) => {
const alignmentId = `${parentLayoutId}-${AlignmentIndexMap[each]}`;
const alignment = document.getElementById(alignmentId);
if (!alignment) return false;
const alignmentWidth = alignment.getBoundingClientRect().width;
// return true if width of any alignment exceeds the limit.
return (
alignmentWidth >= parentLayoutWidth * ALIGNMENT_WIDTH_THRESHOLD
);
});
setIsAnyAlignmentOverflowing(isOverflowing);
});
}
}, [hasFillWidget, layout.length, renderMode]);

useEffect(() => {
if (hasFillWidget || renderMode === RenderModes.PAGE) return;
setIsAnyAlignmentOverflowing(shouldOverrideStyle);
}, [hasFillWidget, renderMode, shouldOverrideStyle]);

const isEditMode = useSelector(isEditOnlyModeSelector);
const isStartVisible = () => startChildren.length > 0 || isEditMode;
const isCenterVisible = () => centerChildren.length > 0 || isEditMode;
const isEndVisible = () => endChildren.length > 0 || isEditMode;

const commonProps: Omit<
FlexLayoutProps,
Expand All @@ -106,21 +44,22 @@ const AlignedWidgetRowComp = (props: LayoutComponentProps) => {
alignSelf: "stretch",
canvasId,
direction: "row",
flexBasis: isAnyAlignmentOverflowing
? { base: "auto" }
: { base: "auto", [`${MOBILE_BREAKPOINT}px`]: "0%" },
flexBasis: "0%",
flexGrow: 1,
flexShrink: 1,
layoutType: LayoutComponentTypes.WIDGET_ROW,
parentDropTarget: props.parentDropTarget,
renderMode: props.renderMode,
wrap: isAnyAlignmentOverflowing
? { base: "wrap" }
: { base: "wrap", [`${MOBILE_BREAKPOINT}px`]: "nowrap" },
renderMode,
wrap: { base: "wrap", [`${MOBILE_BREAKPOINT}px`]: "nowrap" },
className: props.className,
maxWidth: "100%",
};
}, [isAnyAlignmentOverflowing]);
}, []);

// check if layout renders a Fill widget.
const hasFillWidget: boolean = isFillWidgetPresentInList(
layout as WidgetLayoutProps[],
);

// If a Fill widget exists, then render the child widgets together.
if (hasFillWidget) {
Expand Down Expand Up @@ -152,54 +91,48 @@ const AlignedWidgetRowComp = (props: LayoutComponentProps) => {
// WDS Flex can be used as a replacement.
return (
<>
{isStartVisible() && (
<FlexLayout
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was a bad idea, brings less benefit than problems. I'm deleting it.

{...commonProps}
justifyContent="start"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Start]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Start]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.Start]}
>
{renderWidgets({
<FlexLayout
{...commonProps}
justifyContent="start"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Start]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Start]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.Start]}
>
{renderWidgets({
...props,
layout: startChildren,
})}
</FlexLayout>
<FlexLayout
{...commonProps}
justifyContent="center"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Center]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Center]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.Center]}
>
{renderWidgets(
{
...props,
layout: centerChildren,
},
startChildren?.length,
)}
</FlexLayout>
<FlexLayout
{...commonProps}
justifyContent="end"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.End]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.End]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.End]}
>
{renderWidgets(
{
...props,
layout: startChildren,
})}
</FlexLayout>
)}
{isCenterVisible() && (
<FlexLayout
{...commonProps}
justifyContent="center"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Center]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.Center]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.Center]}
>
{renderWidgets(
{
...props,
layout: centerChildren,
},
startChildren?.length,
)}
</FlexLayout>
)}
{isEndVisible() && (
<FlexLayout
{...commonProps}
justifyContent="end"
key={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.End]}`}
layoutId={`${layoutId}-${AlignmentIndexMap[FlexLayerAlignment.End]}`}
layoutIndex={AlignmentIndexMap[FlexLayerAlignment.End]}
>
{renderWidgets(
{
...props,
layout: endChildren,
},
startChildren?.length + centerChildren?.length,
)}
</FlexLayout>
)}
layout: endChildren,
},
startChildren?.length + centerChildren?.length,
)}
</FlexLayout>
</>
);
};
Expand Down
Loading
Loading