From ae396eb9760f941cdb1fce37434c68e6d3297f6b Mon Sep 17 00:00:00 2001 From: Preet Sidhu Date: Thu, 15 Feb 2024 17:02:26 +0530 Subject: [PATCH] fix: clipping issue for excessive hug widgets in an alignment (#31053) ## Description 1. Restores the original behaviour of grow and shrink for alignments, that had regressed after introduction of sizing tokens. 2. Fix clipping issue in alignments. If an alignments width equals or exceeds the width of the row on larger screens, enable flex wrap. #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed ## Summary by CodeRabbit - **New Features** - Introduced a new style property for `AnvilFlexComponent` to enhance layout sizing. - Added a function to determine if default alignment styles need to be overridden. - Launched a new React component for dynamic alignment of widgets within a row, adaptable to different viewport sizes. - **Refactor** - Simplified the rendering process in `AlignedWidgetRow` by improving component organization. - Enhanced logic and structure in rendering functions for better alignment and layout management. - **Bug Fixes** - Fixed incorrect import paths in test and utility files. --- .../anvil/common/AnvilFlexComponent.tsx | 1 + .../anvil/integrations/layoutSelectors.ts | 36 ++++ .../alignedWidgetRow/AlignedWidgetRowComp.tsx | 194 ++++++++++++++++++ .../index.tsx} | 9 +- .../highlights/alignedRowHighlights.test.ts | 2 +- .../anvil/utils/layouts/layoutUtils.ts | 2 +- .../anvil/utils/layouts/renderUtils.tsx | 138 +------------ 7 files changed, 242 insertions(+), 140 deletions(-) create mode 100644 app/client/src/layoutSystems/anvil/integrations/layoutSelectors.ts create mode 100644 app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/AlignedWidgetRowComp.tsx rename app/client/src/layoutSystems/anvil/layoutComponents/components/{AlignedWidgetRow.tsx => alignedWidgetRow/index.tsx} (75%) diff --git a/app/client/src/layoutSystems/anvil/common/AnvilFlexComponent.tsx b/app/client/src/layoutSystems/anvil/common/AnvilFlexComponent.tsx index e62650358e..e74af5aa01 100644 --- a/app/client/src/layoutSystems/anvil/common/AnvilFlexComponent.tsx +++ b/app/client/src/layoutSystems/anvil/common/AnvilFlexComponent.tsx @@ -68,6 +68,7 @@ export const AnvilFlexComponent = forwardRef( flexBasis: isFillWidget ? "0%" : "auto", padding: "spacing-1", alignItems: "center", + width: "max-content", }; if (widgetSize) { const { maxHeight, maxWidth, minHeight, minWidth } = widgetSize; diff --git a/app/client/src/layoutSystems/anvil/integrations/layoutSelectors.ts b/app/client/src/layoutSystems/anvil/integrations/layoutSelectors.ts new file mode 100644 index 0000000000..c4ad61945b --- /dev/null +++ b/app/client/src/layoutSystems/anvil/integrations/layoutSelectors.ts @@ -0,0 +1,36 @@ +import { getLayoutElementPositions } from "layoutSystems/common/selectors"; +import type { + LayoutElementPosition, + LayoutElementPositions, +} from "layoutSystems/common/types"; +import { createSelector } from "reselect"; +import { AlignmentIndexMap } from "../utils/constants"; +import { FlexLayerAlignment } from "layoutSystems/common/utils/constants"; + +export const ALIGNMENT_WIDTH_THRESHOLD = 0.95; + +export function shouldOverrideAlignmentStyle(layoutId: string) { + return createSelector( + getLayoutElementPositions, + (positions: LayoutElementPositions): boolean => { + if (!layoutId || !positions || !positions[layoutId]) return false; + + // If positions don't exist for start alignment, return false as this layout is not aligned. + if (!positions[`${layoutId}-0`]) return false; + + const layoutPosition: LayoutElementPosition = positions[layoutId]; + const threshold = layoutPosition.width * ALIGNMENT_WIDTH_THRESHOLD; + + // return true if width of any alignment exceeds the limit. + return [ + FlexLayerAlignment.Start, + FlexLayerAlignment.Center, + FlexLayerAlignment.End, + ].some((each: FlexLayerAlignment) => { + const alignmentPosition: LayoutElementPosition = + positions[`${layoutId}-${AlignmentIndexMap[each]}`]; + return alignmentPosition.width >= threshold; + }); + }, + ); +} diff --git a/app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/AlignedWidgetRowComp.tsx b/app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/AlignedWidgetRowComp.tsx new file mode 100644 index 0000000000..a4e9905e38 --- /dev/null +++ b/app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/AlignedWidgetRowComp.tsx @@ -0,0 +1,194 @@ +import React, { useEffect, useMemo, useState } from "react"; +import { + LayoutComponentTypes, + type LayoutComponentProps, + type WidgetLayoutProps, +} from "layoutSystems/anvil/utils/anvilTypes"; +import { + AlignmentIndexMap, + MOBILE_BREAKPOINT, +} from "layoutSystems/anvil/utils/constants"; +import { FlexLayerAlignment } from "layoutSystems/common/utils/constants"; +import { renderWidgets } from "layoutSystems/anvil/utils/layouts/renderUtils"; +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: + * then render all children directly within the AlignedRow (row / flex-start / wrap); + * no need for alignments. + * + * Else: + * render children in 3 alignments: start, center and end. + * 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. + * ~ 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); + 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 commonProps: Omit< + FlexLayoutProps, + "children" | "layoutId" | "layoutIndex" + > = useMemo(() => { + return { + alignSelf: "stretch", + canvasId, + direction: "row", + flexBasis: isAnyAlignmentOverflowing + ? { base: "auto" } + : { base: "auto", [`${MOBILE_BREAKPOINT}px`]: "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" }, + className: props.className, + }; + }, [isAnyAlignmentOverflowing]); + + // If a Fill widget exists, then render the child widgets together. + if (hasFillWidget) { + return <>{renderWidgets(props)}; + } + + /** + * else render the child widgets separately + * in their respective alignments. + */ + const startChildren: WidgetLayoutProps[] = ( + layout as WidgetLayoutProps[] + ).filter( + (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.Start, + ); + const centerChildren: WidgetLayoutProps[] = ( + layout as WidgetLayoutProps[] + ).filter( + (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.Center, + ); + const endChildren: WidgetLayoutProps[] = ( + layout as WidgetLayoutProps[] + ).filter( + (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.End, + ); + + // TODO: After positionObserver integration, + // check if use of FlexLayout is causing performance or other issues. + // WDS Flex can be used as a replacement. + return ( + <> + + {renderWidgets({ + ...props, + layout: startChildren, + })} + + + {renderWidgets( + { + ...props, + layout: centerChildren, + }, + startChildren?.length, + )} + + + {renderWidgets( + { + ...props, + layout: endChildren, + }, + startChildren?.length + centerChildren?.length, + )} + + + ); +}; + +export default AlignedWidgetRowComp; diff --git a/app/client/src/layoutSystems/anvil/layoutComponents/components/AlignedWidgetRow.tsx b/app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/index.tsx similarity index 75% rename from app/client/src/layoutSystems/anvil/layoutComponents/components/AlignedWidgetRow.tsx rename to app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/index.tsx index f1e9713019..50abb014c2 100644 --- a/app/client/src/layoutSystems/anvil/layoutComponents/components/AlignedWidgetRow.tsx +++ b/app/client/src/layoutSystems/anvil/layoutComponents/components/alignedWidgetRow/index.tsx @@ -1,11 +1,12 @@ -import BaseLayoutComponent from "../BaseLayoutComponent"; +import React from "react"; +import BaseLayoutComponent from "../../BaseLayoutComponent"; import { type DeriveHighlightsFn, LayoutComponentTypes, } from "layoutSystems/anvil/utils/anvilTypes"; -import type { FlexLayoutProps } from "./FlexLayout"; +import type { FlexLayoutProps } from "../FlexLayout"; import { deriveAlignedRowHighlights } from "layoutSystems/anvil/utils/layouts/highlights/alignedRowHighlights"; -import { renderWidgetsInAlignedRow } from "layoutSystems/anvil/utils/layouts/renderUtils"; +import AlignedWidgetRowComp from "./AlignedWidgetRowComp"; class AlignedWidgetRow extends BaseLayoutComponent { static type: LayoutComponentTypes = LayoutComponentTypes.ALIGNED_WIDGET_ROW; @@ -13,7 +14,7 @@ class AlignedWidgetRow extends BaseLayoutComponent { static deriveHighlights: DeriveHighlightsFn = deriveAlignedRowHighlights; renderChildWidgets(): React.ReactNode { - return renderWidgetsInAlignedRow(this.props); + return ; } static rendersWidgets: boolean = true; diff --git a/app/client/src/layoutSystems/anvil/utils/layouts/highlights/alignedRowHighlights.test.ts b/app/client/src/layoutSystems/anvil/utils/layouts/highlights/alignedRowHighlights.test.ts index cb3711d848..6f8c08fb3d 100644 --- a/app/client/src/layoutSystems/anvil/utils/layouts/highlights/alignedRowHighlights.test.ts +++ b/app/client/src/layoutSystems/anvil/utils/layouts/highlights/alignedRowHighlights.test.ts @@ -11,7 +11,7 @@ import { } from "layoutSystems/common/utils/constants"; import { HIGHLIGHT_SIZE } from "../../constants"; import LayoutFactory from "layoutSystems/anvil/layoutComponents/LayoutFactory"; -import AlignedWidgetRow from "layoutSystems/anvil/layoutComponents/components/AlignedWidgetRow"; +import AlignedWidgetRow from "layoutSystems/anvil/layoutComponents/components/alignedWidgetRow"; import type { BaseWidgetProps } from "widgets/BaseWidgetHOC/withBaseWidgetHOC"; import { mockButtonProps } from "mocks/widgetProps/button"; import { getAlignmentLayoutId } from "../layoutUtils"; diff --git a/app/client/src/layoutSystems/anvil/utils/layouts/layoutUtils.ts b/app/client/src/layoutSystems/anvil/utils/layouts/layoutUtils.ts index 7c2df6f0fd..71658d8dd3 100644 --- a/app/client/src/layoutSystems/anvil/utils/layouts/layoutUtils.ts +++ b/app/client/src/layoutSystems/anvil/utils/layouts/layoutUtils.ts @@ -9,7 +9,7 @@ import type { FlexLayerAlignment } from "layoutSystems/common/utils/constants"; import { AlignmentIndexMap } from "../constants"; import AlignedLayoutColumn from "layoutSystems/anvil/layoutComponents/components/AlignedLayoutColumn"; import AlignedWidgetColumn from "layoutSystems/anvil/layoutComponents/components/AlignedWidgetColumn"; -import AlignedWidgetRow from "layoutSystems/anvil/layoutComponents/components/AlignedWidgetRow"; +import AlignedWidgetRow from "layoutSystems/anvil/layoutComponents/components/alignedWidgetRow"; import LayoutColumn from "layoutSystems/anvil/layoutComponents/components/LayoutColumn"; import LayoutRow from "layoutSystems/anvil/layoutComponents/components/LayoutRow"; import WidgetColumn from "layoutSystems/anvil/layoutComponents/components/WidgetColumn"; diff --git a/app/client/src/layoutSystems/anvil/utils/layouts/renderUtils.tsx b/app/client/src/layoutSystems/anvil/utils/layouts/renderUtils.tsx index bba5a3c9ea..77adea5a24 100644 --- a/app/client/src/layoutSystems/anvil/utils/layouts/renderUtils.tsx +++ b/app/client/src/layoutSystems/anvil/utils/layouts/renderUtils.tsx @@ -1,19 +1,11 @@ import React, { type ReactNode } from "react"; import LayoutFactory from "layoutSystems/anvil/layoutComponents/LayoutFactory"; -import { - LayoutComponentTypes, - type LayoutComponentProps, - type LayoutProps, - type WidgetLayoutProps, +import type { + LayoutComponentProps, + LayoutProps, + WidgetLayoutProps, } from "../anvilTypes"; import { type RenderMode, RenderModes } from "constants/WidgetConstants"; -import { isFillWidgetPresentInList } from "./widgetUtils"; -import { AlignmentIndexMap, MOBILE_BREAKPOINT } from "../constants"; -import { - FlexLayout, - type FlexLayoutProps, -} from "layoutSystems/anvil/layoutComponents/components/FlexLayout"; -import { FlexLayerAlignment } from "layoutSystems/common/utils/constants"; import type BaseLayoutComponent from "layoutSystems/anvil/layoutComponents/BaseLayoutComponent"; import { WidgetRenderer } from "layoutSystems/anvil/layoutComponents/WidgetRenderer"; @@ -78,125 +70,3 @@ export function renderLayouts( ); }); } - -/** - * If AlignedRow hasFillWidget: - * then render all children directly within the AlignedRow (row / flex-start / wrap); - * no need for alignments. - * - * Else: - * render children in 3 alignments: start, center and end. - * 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. - * ~ 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. - */ -export function renderWidgetsInAlignedRow( - props: LayoutComponentProps, -): React.ReactNode { - const { canvasId, layout, layoutId } = props; - // 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) { - return renderWidgets(props); - } - - /** - * else render the child widgets separately - * in their respective alignments. - */ - const commonProps: Omit< - FlexLayoutProps, - "children" | "layoutId" | "layoutIndex" - > = { - alignSelf: "stretch", - canvasId, - direction: "row", - flexBasis: { base: "auto", [`${MOBILE_BREAKPOINT}px`]: "0%" }, - flexGrow: 1, - flexShrink: 1, - layoutType: LayoutComponentTypes.WIDGET_ROW, - parentDropTarget: props.parentDropTarget, - renderMode: props.renderMode, - wrap: { base: "wrap", [`${MOBILE_BREAKPOINT}px`]: "nowrap" }, - className: props.className, - }; - - const startChildren: WidgetLayoutProps[] = ( - layout as WidgetLayoutProps[] - ).filter( - (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.Start, - ); - const centerChildren: WidgetLayoutProps[] = ( - layout as WidgetLayoutProps[] - ).filter( - (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.Center, - ); - const endChildren: WidgetLayoutProps[] = ( - layout as WidgetLayoutProps[] - ).filter( - (each: WidgetLayoutProps) => each.alignment === FlexLayerAlignment.End, - ); - - // TODO: After positionObserver integration, - // check if use of FlexLayout is causing performance or other issues. - // WDS Flex can be used as a replacement. - return [ - - {renderWidgets({ - ...props, - layout: startChildren, - })} - , - - {renderWidgets( - { - ...props, - layout: centerChildren, - }, - startChildren?.length, - )} - , - - {renderWidgets( - { - ...props, - layout: endChildren, - }, - startChildren?.length + centerChildren?.length, - )} - , - ]; -}