From f7be484cf4176a7431613bf15308e85cb6f30df6 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 13 May 2024 13:15:33 -0500 Subject: [PATCH 1/5] refactor(Stack): write to single custom property for gap changes --- packages/react/src/Stack/Stack.tsx | 38 +++++++++++++++++------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/react/src/Stack/Stack.tsx b/packages/react/src/Stack/Stack.tsx index f423ce4fce1..e18fdff4fd0 100644 --- a/packages/react/src/Stack/Stack.tsx +++ b/packages/react/src/Stack/Stack.tsx @@ -4,15 +4,16 @@ import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {getResponsiveAttributes} from '../internal/utils/getResponsiveAttributes' const StyledStack = styled.div` - --Stack-gap-whenRegular: var(--stack-gap-normal, 16px); - --Stack-gap-whenNarrow: var(--stack-gap-normal, 16px); - --Stack-gap-whenWide: var(--Stack-gap-whenRegular); + --stack-gap-none: 0; + --stack-gap-condensed: 0.5rem; + --stack-gap-normal: 1rem; + --stack-gap-spacious: 2rem; display: flex; flex-flow: column; align-items: stretch; align-content: flex-start; - gap: var(--Stack-gap-whenNarrow); + gap: var(--stack-gap, var(--stack-gap-normal)); // non-responsive values @@ -48,17 +49,22 @@ const StyledStack = styled.div` &[data-gap='none'], &[data-gap-narrow='none'] { - --Stack-gap-whenNarrow: 0; + --stack-gap: var(--stack-gap-none); } &[data-gap='condensed'], &[data-gap-narrow='condensed'] { - --Stack-gap-whenNarrow: var(--stack-gap-condensed, 8px); + --stack-gap: var(--stack-gap-condensed); } &[data-gap='normal'], &[data-gap-narrow='normal'] { - --Stack-gap-whenNarrow: var(--stack-gap-normal, 16px); + --stack-gap: var(--stack-gap-normal); + } + + &[data-gap='spacious'], + &[data-gap-narrow='spacious'] { + --stack-gap: var(--stack-gap-spacious); } &[data-align='start'], @@ -143,19 +149,19 @@ const StyledStack = styled.div` } &[data-gap-regular='none'] { - --Stack-gap-whenRegular: 0; + --stack-gap: var(--stack-gap-none); } &[data-gap-regular='condensed'] { - --Stack-gap-whenRegular: var(--stack-gap-condensed, 8px); + --stack-gap: var(--stack-gap-condensed); } &[data-gap-regular='normal'] { - --Stack-gap-whenRegular: var(--stack-gap-normal, 16px); + --stack-gap: var(--stack-gap-normal); } &[data-gap-regular='spacious'] { - --Stack-gap-whenRegular: var(--stack-gap-spacious, 24px); + --stack-gap: var(--stack-gap-spacious); } &[data-align-regular='start'] { @@ -205,8 +211,6 @@ const StyledStack = styled.div` // @custom-media --viewportRange-wide @media (min-width: 87.5rem) { - gap: var(--Stack-gap-whenWide); - &[data-padding-wide='none'] { padding: 0; } @@ -232,19 +236,19 @@ const StyledStack = styled.div` } &[data-gap-wide='none'] { - --Stack-gap-whenWide: 0; + --stack-gap: var(--stack-gap-none); } &[data-gap-wide='condensed'] { - --Stack-gap-whenWide: var(--stack-gap-condensed, 8px); + --stack-gap: var(--stack-gap-condensed); } &[data-gap-wide='normal'] { - --Stack-gap-whenWide: var(--stack-gap-normal, 16px); + --stack-gap: var(--stack-gap-normal); } &[data-gap-wide='spacious'] { - --Stack-gap-whenWide: var(--stack-gap-spacious, 24px); + --stack-gap: var(--stack-gap-spacious); } &[data-align-wide='start'] { From 48d68229760177221430920837dffc4c07d66603 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 13 May 2024 13:18:33 -0500 Subject: [PATCH 2/5] chore: add changeset --- .changeset/silent-apricots-fly.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/silent-apricots-fly.md diff --git a/.changeset/silent-apricots-fly.md b/.changeset/silent-apricots-fly.md new file mode 100644 index 00000000000..e5516655b6e --- /dev/null +++ b/.changeset/silent-apricots-fly.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Update how gap is set in Stack to work in wide breakpoints From 02f3c4b86431d0f1e217373fc83ef440dca6660f Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 13 May 2024 14:02:22 -0500 Subject: [PATCH 3/5] test: update style test --- packages/react/src/Stack/__tests__/Stack.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Stack/__tests__/Stack.test.tsx b/packages/react/src/Stack/__tests__/Stack.test.tsx index d4a361aeca5..dacea141577 100644 --- a/packages/react/src/Stack/__tests__/Stack.test.tsx +++ b/packages/react/src/Stack/__tests__/Stack.test.tsx @@ -98,7 +98,7 @@ describe('Stack', () => { describe('gap', () => { it('should set the default gap to `normal`', () => { render() - expect(screen.getByTestId('stack')).toHaveStyle('gap: var(--Stack-gap-whenNarrow);') + expect(screen.getByTestId('stack')).toHaveStyle('gap: var(--stack-gap,var(--stack-gap-normal));') }) it('should support specifying the stack gap with the `gap` prop', () => { From 320ee0603260a4db36a05b65dc70dcf07510a251 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 14 May 2024 10:24:30 -0500 Subject: [PATCH 4/5] refactor(Stack): use primitive tokens directly with fallback value --- packages/react/src/Stack/Stack.tsx | 31 +++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/react/src/Stack/Stack.tsx b/packages/react/src/Stack/Stack.tsx index e18fdff4fd0..e42ae4c2825 100644 --- a/packages/react/src/Stack/Stack.tsx +++ b/packages/react/src/Stack/Stack.tsx @@ -4,16 +4,11 @@ import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {getResponsiveAttributes} from '../internal/utils/getResponsiveAttributes' const StyledStack = styled.div` - --stack-gap-none: 0; - --stack-gap-condensed: 0.5rem; - --stack-gap-normal: 1rem; - --stack-gap-spacious: 2rem; - display: flex; flex-flow: column; align-items: stretch; align-content: flex-start; - gap: var(--stack-gap, var(--stack-gap-normal)); + gap: var(--stack-gap, var(--stack-gap-normal, 1rem)); // non-responsive values @@ -49,22 +44,22 @@ const StyledStack = styled.div` &[data-gap='none'], &[data-gap-narrow='none'] { - --stack-gap: var(--stack-gap-none); + --stack-gap: var(--stack-gap-none, 0); } &[data-gap='condensed'], &[data-gap-narrow='condensed'] { - --stack-gap: var(--stack-gap-condensed); + --stack-gap: var(--stack-gap-condensed, 0.5rem); } &[data-gap='normal'], &[data-gap-narrow='normal'] { - --stack-gap: var(--stack-gap-normal); + --stack-gap: var(--stack-gap-normal, 1rem); } &[data-gap='spacious'], &[data-gap-narrow='spacious'] { - --stack-gap: var(--stack-gap-spacious); + --stack-gap: var(--stack-gap-spacious, 1.5rem); } &[data-align='start'], @@ -149,19 +144,19 @@ const StyledStack = styled.div` } &[data-gap-regular='none'] { - --stack-gap: var(--stack-gap-none); + --stack-gap: var(--stack-gap-none, 0); } &[data-gap-regular='condensed'] { - --stack-gap: var(--stack-gap-condensed); + --stack-gap: var(--stack-gap-condensed, 0.5rem); } &[data-gap-regular='normal'] { - --stack-gap: var(--stack-gap-normal); + --stack-gap: var(--stack-gap-normal, 1rem); } &[data-gap-regular='spacious'] { - --stack-gap: var(--stack-gap-spacious); + --stack-gap: var(--stack-gap-spacious, 1.5rem); } &[data-align-regular='start'] { @@ -236,19 +231,19 @@ const StyledStack = styled.div` } &[data-gap-wide='none'] { - --stack-gap: var(--stack-gap-none); + --stack-gap: var(--stack-gap-none, 0); } &[data-gap-wide='condensed'] { - --stack-gap: var(--stack-gap-condensed); + --stack-gap: var(--stack-gap-condensed, 0.5rem); } &[data-gap-wide='normal'] { - --stack-gap: var(--stack-gap-normal); + --stack-gap: var(--stack-gap-normal, 1rem); } &[data-gap-wide='spacious'] { - --stack-gap: var(--stack-gap-spacious); + --stack-gap: var(--stack-gap-spacious, 1.5rem); } &[data-align-wide='start'] { From 5a587f8301c86f01d03725f06f4a8a9eb3367859 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 16 May 2024 11:44:40 -0500 Subject: [PATCH 5/5] test(Stack): update test with new fallback in custom property --- packages/react/src/Stack/__tests__/Stack.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Stack/__tests__/Stack.test.tsx b/packages/react/src/Stack/__tests__/Stack.test.tsx index dacea141577..e4cca6c27d0 100644 --- a/packages/react/src/Stack/__tests__/Stack.test.tsx +++ b/packages/react/src/Stack/__tests__/Stack.test.tsx @@ -98,7 +98,7 @@ describe('Stack', () => { describe('gap', () => { it('should set the default gap to `normal`', () => { render() - expect(screen.getByTestId('stack')).toHaveStyle('gap: var(--stack-gap,var(--stack-gap-normal));') + expect(screen.getByTestId('stack')).toHaveStyle('gap: var(--stack-gap,var(--stack-gap-normal,1rem));') }) it('should support specifying the stack gap with the `gap` prop', () => {