From 889b9140fef2f610fb34a5d2d2fed9d592c63f2c Mon Sep 17 00:00:00 2001 From: Mark Higham Date: Mon, 19 Aug 2024 10:10:41 +0100 Subject: [PATCH] fix(Group): Use flexbox styles for growing group child elements (#3863) --- .../src/components/Group/Group.stories.tsx | 58 ++----------------- .../src/components/Group/Group.test.tsx | 20 +------ .../src/components/Group/Group.tsx | 22 ++----- 3 files changed, 11 insertions(+), 89 deletions(-) diff --git a/packages/react-component-library/src/components/Group/Group.stories.tsx b/packages/react-component-library/src/components/Group/Group.stories.tsx index e3d440a484..c7d3d6427f 100644 --- a/packages/react-component-library/src/components/Group/Group.stories.tsx +++ b/packages/react-component-library/src/components/Group/Group.stories.tsx @@ -1,9 +1,9 @@ import React from 'react' -import styled from 'styled-components' import { Meta, StoryFn } from '@storybook/react' import { Group } from '.' import { Button } from '../Button' +import { getSpacings } from '../../tokens/utils' export default { component: Group, @@ -18,12 +18,7 @@ export default { }, argTypes: { gap: { - options: [ - ...Array.from({ length: 21 }, (_, i) => i.toString()), - 'px', - 'half', - 'full', - ], + options: [...getSpacings(), 'px', 'half', 'full'], }, justify: { options: [ @@ -41,6 +36,9 @@ export default { wrap: { options: ['wrap', 'nowrap', 'wrap-reverse'], }, + grow: { + control: 'boolean', + }, }, } as Meta @@ -57,49 +55,3 @@ Default.args = { justify: 'flex-start', grow: false, } - -const StyledButton = styled(Button)` - overflow: hidden; -` - -const StyledP = styled.p` - margin: 1rem 0; -` - -export const PreventGrowOverflow: StoryFn = (_props) => ( -
- - preventGrowOverflow: true – each child width is always limited to - 33% of parent width (since there are 3 children) - - - - - - Second button with large content - - - - - - preventGrowOverflow: false – children will grow based on their - content, they can take more than 33% of parent width - - - - - - - -
-) - -PreventGrowOverflow.storyName = 'preventGrowOverflow' -PreventGrowOverflow.parameters = { - docs: { - description: { - story: - 'preventGrowOverflow prop allows you to control how Group children should behave when there is not enough space to fit them all on one line. By default, children are not allowed to take more space than (1 / children.length) * 100% of parent width (preventGrowOverflow is set to true). To change this behavior, set preventGrowOverflow to false and children will be allowed to grow and take as much space as they need.', - }, - }, -} diff --git a/packages/react-component-library/src/components/Group/Group.test.tsx b/packages/react-component-library/src/components/Group/Group.test.tsx index 6f2abb1718..968d3a9a8b 100644 --- a/packages/react-component-library/src/components/Group/Group.test.tsx +++ b/packages/react-component-library/src/components/Group/Group.test.tsx @@ -39,29 +39,13 @@ describe('Group', () => { expect(container.firstChild).toHaveStyleRule('flex-wrap', 'nowrap') }) - it('apply flex-grow to children when grow is true', () => { + it('apply flex:1 to children when grow is true', () => { const { container } = render(Content) - expect(container.firstChild).toHaveStyleRule('flex-grow', '1', { + expect(container.firstChild).toHaveStyleRule('flex', '1', { modifier: '> *', }) }) - it('apply max-width to children when preventGrowOverflow is true', () => { - const customGap = '2' - const { container } = render( - -
Child 1
-
Child 2
-
Child 3
-
- ) - expect(container.firstChild).toHaveStyleRule( - 'max-width', - `calc(33.333333333333336% - (${spacing(customGap)} / 3))`, - { modifier: '> *' } - ) - }) - it('render with the specified HTML element', () => { const { container } = render(Content) expect(container?.firstChild?.nodeName).toBe('SECTION') diff --git a/packages/react-component-library/src/components/Group/Group.tsx b/packages/react-component-library/src/components/Group/Group.tsx index eab8fa912e..b1b07bbbd3 100644 --- a/packages/react-component-library/src/components/Group/Group.tsx +++ b/packages/react-component-library/src/components/Group/Group.tsx @@ -14,11 +14,11 @@ interface GroupProps extends ComponentWithClass { */ gap?: Spacing /** - * Controls `justify-content` CSS property, `'flex-start'` by default. + * Controls horizontal `justify-content` CSS property, `'flex-start'` by default. */ justify?: CSS.Properties['justifyContent'] /** - * Controls `align-items` CSS property, `'center'` by default. + * Controls vertical `align-items` CSS property, `'center'` by default. */ align?: CSS.Properties['alignItems'] /** @@ -30,12 +30,6 @@ interface GroupProps extends ComponentWithClass { * style, `false` by default. */ grow?: boolean - /** - * Determines whether children should take only dedicated amount of - * space (`max-width` style is set based on the number of children) - * , `true` by default. - */ - preventGrowOverflow?: boolean /** * The type of element to use for the root node, `'div'` by default. */ @@ -55,17 +49,11 @@ const StyledGroup = styled.div` justify-content: ${({ $justify }) => $justify}; flex-wrap: ${({ $wrap }) => $wrap}; - ${({ $grow, $gap, $preventGrowOverflow }) => + ${({ $grow }) => $grow && css` > * { - flex-grow: 1; - ${$preventGrowOverflow && - css` - max-width: ${$gap === '0' - ? '33.333333333333336%' - : `calc(33.333333333333336% - (${spacing($gap!)} / 3))`}; - `} + flex: 1; } `} ` @@ -79,7 +67,6 @@ export const Group = ({ justify = 'flex-start', wrap = 'wrap', grow = false, - preventGrowOverflow = true, }: GroupProps) => { return ( children && ( @@ -91,7 +78,6 @@ export const Group = ({ $justify={justify} $wrap={wrap} $grow={grow} - $preventGrowOverflow={preventGrowOverflow} > {children}