Skip to content

Commit

Permalink
refactor(Stack): write to single custom property for gap changes (#4589)
Browse files Browse the repository at this point in the history
* refactor(Stack): write to single custom property for gap changes

* chore: add changeset

* test: update style test

* refactor(Stack): use primitive tokens directly with fallback value

* test(Stack): update test with new fallback in custom property

---------

Co-authored-by: Josh Black <[email protected]>
  • Loading branch information
2 people authored and khiga8 committed May 31, 2024
1 parent ac16f4e commit d67528c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/silent-apricots-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Update how gap is set in Stack to work in wide breakpoints
35 changes: 17 additions & 18 deletions packages/react/src/Stack/Stack.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@ 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);
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, 1rem));
// non-responsive values
Expand Down Expand Up @@ -48,17 +44,22 @@ const StyledStack = styled.div`
&[data-gap='none'],
&[data-gap-narrow='none'] {
--Stack-gap-whenNarrow: 0;
--stack-gap: var(--stack-gap-none, 0);
}
&[data-gap='condensed'],
&[data-gap-narrow='condensed'] {
--Stack-gap-whenNarrow: var(--stack-gap-condensed, 8px);
--stack-gap: var(--stack-gap-condensed, 0.5rem);
}
&[data-gap='normal'],
&[data-gap-narrow='normal'] {
--Stack-gap-whenNarrow: var(--stack-gap-normal, 16px);
--stack-gap: var(--stack-gap-normal, 1rem);
}
&[data-gap='spacious'],
&[data-gap-narrow='spacious'] {
--stack-gap: var(--stack-gap-spacious, 1.5rem);
}
&[data-align='start'],
Expand Down Expand Up @@ -143,19 +144,19 @@ const StyledStack = styled.div`
}
&[data-gap-regular='none'] {
--Stack-gap-whenRegular: 0;
--stack-gap: var(--stack-gap-none, 0);
}
&[data-gap-regular='condensed'] {
--Stack-gap-whenRegular: var(--stack-gap-condensed, 8px);
--stack-gap: var(--stack-gap-condensed, 0.5rem);
}
&[data-gap-regular='normal'] {
--Stack-gap-whenRegular: var(--stack-gap-normal, 16px);
--stack-gap: var(--stack-gap-normal, 1rem);
}
&[data-gap-regular='spacious'] {
--Stack-gap-whenRegular: var(--stack-gap-spacious, 24px);
--stack-gap: var(--stack-gap-spacious, 1.5rem);
}
&[data-align-regular='start'] {
Expand Down Expand Up @@ -205,8 +206,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;
}
Expand All @@ -232,19 +231,19 @@ const StyledStack = styled.div`
}
&[data-gap-wide='none'] {
--Stack-gap-whenWide: 0;
--stack-gap: var(--stack-gap-none, 0);
}
&[data-gap-wide='condensed'] {
--Stack-gap-whenWide: var(--stack-gap-condensed, 8px);
--stack-gap: var(--stack-gap-condensed, 0.5rem);
}
&[data-gap-wide='normal'] {
--Stack-gap-whenWide: var(--stack-gap-normal, 16px);
--stack-gap: var(--stack-gap-normal, 1rem);
}
&[data-gap-wide='spacious'] {
--Stack-gap-whenWide: var(--stack-gap-spacious, 24px);
--stack-gap: var(--stack-gap-spacious, 1.5rem);
}
&[data-align-wide='start'] {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Stack/__tests__/Stack.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('Stack', () => {
describe('gap', () => {
it('should set the default gap to `normal`', () => {
render(<Stack data-testid="stack" />)
expect(screen.getByTestId('stack')).toHaveStyle('gap: var(--Stack-gap-whenNarrow);')
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', () => {
Expand Down

0 comments on commit d67528c

Please sign in to comment.