Skip to content

Commit

Permalink
Merge branch 'main' into mp/labelgroup-truncation
Browse files Browse the repository at this point in the history
  • Loading branch information
mperrotti authored Jul 28, 2023
2 parents 923f605 + d8bbbb3 commit 674274b
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 76 deletions.
7 changes: 7 additions & 0 deletions .changeset/hot-planes-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': patch
---

Css styled block fixes

<!-- Changed components: DataTable, Timeline -->
17 changes: 15 additions & 2 deletions .changeset/hungry-spies-remember.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
"@primer/react": patch
---

Deprecates position prop for PageLayout.Pane. Users will receive a console warning when it is used.
Deprecates `position` prop for PageLayout.Pane and SplitPageLayout.Pane.

<!-- Changed components: PageLayout -->
```diff
-<PageLayout>
- <PageLayout.Content />
- <PageLayout.Pane position="start" />
-</PageLayout>

+<PageLayout>
+ <PageLayout.Pane />
+ <PageLayout.Content />
+</PageLayout>

```

<!-- Changed components: PageLayout, SplitPageLayout -->
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 10 additions & 10 deletions docs/content/PageLayout.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ See [storybook](https://primer.style/react/storybook?path=/story/components-page
<PageLayout.Header>
<Placeholder label="Header" height={64} />
</PageLayout.Header>
<PageLayout.Pane>
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Content>
<Placeholder label="Content" height={240} />
</PageLayout.Content>
<PageLayout.Pane position="start">
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Footer>
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand All @@ -106,12 +106,12 @@ See [storybook](https://primer.style/react/storybook?path=/story/components-page
<PageLayout.Header>
<Placeholder label="Header" height={64} />
</PageLayout.Header>
<PageLayout.Pane hidden={{narrow: true}}>
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Content>
<Placeholder label="Content" height={240} />
</PageLayout.Content>
<PageLayout.Pane position="start" hidden={{narrow: true}}>
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Footer>
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand Down Expand Up @@ -193,12 +193,12 @@ Add `offsetHeader` prop to specify the height of the custom sticky header along
Custom sticky header
</Box>
<PageLayout>
<PageLayout.Pane sticky offsetHeader={64}>
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Content>
<Placeholder label="Content" height={320} />
</PageLayout.Content>
<PageLayout.Pane position="start" sticky offsetHeader={64}>
<Placeholder label="Pane" height={120} />
</PageLayout.Pane>
<PageLayout.Footer>
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand All @@ -221,7 +221,7 @@ navigation container is used for.
<PageLayout.Header>
<Placeholder label="Header" height={64} />
</PageLayout.Header>
<PageLayout.Pane position="start" aria-label="Secondary navigation">
<PageLayout.Pane aria-label="Secondary navigation">
<NavList>
<NavList.Item href="/" aria-current="page">
Home
Expand Down
2 changes: 1 addition & 1 deletion docs/content/SplitPageLayout.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ If you need a more flexible layout component, consider using the [PageLayout](/P
<SplitPageLayout.Content>
<Placeholder label="Content" height={420} />
</SplitPageLayout.Content>
<SplitPageLayout.Pane position="end">
<SplitPageLayout.Pane>
<Placeholder label="Pane" height={120} />
</SplitPageLayout.Pane>
<SplitPageLayout.Footer>
Expand Down
1 change: 1 addition & 0 deletions src/DataTable/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ const StyledTable = styled.table<React.ComponentPropsWithoutRef<'table'>>`
display: grid;
grid-template-columns: subgrid;
grid-column: -1 /1;
}
}
`

Expand Down
2 changes: 1 addition & 1 deletion src/PageLayout/PageLayout.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
"defaultValue": "'end'",
"description": "",
"description": "Use source order instead of relying on the `position` prop",
"deprecated": true
},
{
Expand Down
26 changes: 19 additions & 7 deletions src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,25 @@ Content.displayName = 'PageLayout.Content'
// PageLayout.Pane

export type PageLayoutPaneProps = {
/**
* @deprecated Use source order instead of relying on the `position` prop
*
* Before:
* ```
* <PageLayout>
* <PageLayout.Content />
* <PageLayout.Pane position="start" />
* </PageLayout>
* ```
*
* After:
* ```
* <PageLayout>
* <PageLayout.Pane />
* <PageLayout.Content />
* </PageLayout>
* ```
*/
position?: keyof typeof panePositions | ResponsiveValue<keyof typeof panePositions>
/**
* @deprecated Use the `position` prop with a responsive value instead.
Expand Down Expand Up @@ -563,13 +582,6 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
},
forwardRef,
) => {
if (responsivePosition !== undefined) {
// eslint-disable-next-line no-console
console.warn(
'The `position` prop will be removed on the next major version. You should order your markup as you want it to render instead.',
)
}

// Combine position and positionWhenNarrow for backwards compatibility
const positionProp =
!isResponsiveValue(responsivePosition) && positionWhenNarrow !== 'inherit'
Expand Down
2 changes: 1 addition & 1 deletion src/SplitPageLayout/SplitPageLayout.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
"defaultValue": "start'",
"description": "",
"description": "Use source order instead of relying on the `position` prop",
"deprecated": true
},
{
Expand Down
4 changes: 0 additions & 4 deletions src/SplitPageLayout/SplitPageLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ describe('SplitPageLayout', () => {
})

it('renders default layout', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
const {container} = render(
<ThemeProvider>
<SplitPageLayout>
Expand All @@ -28,21 +27,18 @@ describe('SplitPageLayout', () => {
</SplitPageLayout>
</ThemeProvider>,
)
expect(spy).toHaveBeenCalled()

expect(container).toMatchSnapshot()
})

it('renders Pane with a custom ID', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
const {getByText} = render(
<ThemeProvider>
<SplitPageLayout>
<SplitPageLayout.Pane id="customId">Pane Content</SplitPageLayout.Pane>
</SplitPageLayout>
</ThemeProvider>,
)
expect(spy).toHaveBeenCalled()

const pane = getByText('Pane Content')

Expand Down
2 changes: 1 addition & 1 deletion src/Timeline/Timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const TimelineBody = styled.div<SxProp>`
`

const TimelineBreak = styled.div<SxProp>`
position: relative
position: relative;
z-index: 1;
height: 24px;
margin: 0;
Expand Down
13 changes: 1 addition & 12 deletions src/UnderlineNav2/UnderlineNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -310,18 +310,7 @@ export const UnderlineNav = forwardRef(
ref={navRef}
>
<NavigationList sx={ulStyles} ref={listRef} role="list">
{listItems.map(listItem => {
return (
<Box
key={listItem.props.children}
as="li"
sx={{display: 'flex', flexDirection: 'column', alignItems: 'center'}}
>
{listItem}
</Box>
)
})}

{listItems}
{menuItems.length > 0 && (
<MoreMenuListItem ref={moreMenuRef}>
{!onlyMenuVisible && <Box sx={getDividerStyle(theme)}></Box>}
Expand Down
72 changes: 37 additions & 35 deletions src/UnderlineNav2/UnderlineNavItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,43 +114,45 @@ export const UnderlineNavItem = forwardRef(
)

return (
<Link
ref={ref}
as={Component}
href={href}
aria-current={ariaCurrent}
onKeyDown={keyDownHandler}
onClick={clickHandler}
sx={merge<BetterSystemStyleObject>(getLinkStyles(theme, ariaCurrent), sxProp as SxProp)}
{...props}
>
{iconsVisible && Icon && (
<Box as="span" data-component="icon" sx={iconWrapStyles}>
<Icon />
</Box>
)}
{children && (
<Box
as="span"
data-component="text"
data-content={children}
sx={Boolean(ariaCurrent) && ariaCurrent !== 'false' ? {fontWeight: 600} : {}}
>
{children}
</Box>
)}
{loadingCounters ? (
<Box as="span" data-component="counter" sx={counterStyles}>
<LoadingCounter />
</Box>
) : (
counter !== undefined && (
<Box as="li" sx={{display: 'flex', flexDirection: 'column', alignItems: 'center'}}>
<Link
ref={ref}
as={Component}
href={href}
aria-current={ariaCurrent}
onKeyDown={keyDownHandler}
onClick={clickHandler}
sx={merge<BetterSystemStyleObject>(getLinkStyles(theme, ariaCurrent), sxProp as SxProp)}
{...props}
>
{iconsVisible && Icon && (
<Box as="span" data-component="icon" sx={iconWrapStyles}>
<Icon />
</Box>
)}
{children && (
<Box
as="span"
data-component="text"
data-content={children}
sx={Boolean(ariaCurrent) && ariaCurrent !== 'false' ? {fontWeight: 600} : {}}
>
{children}
</Box>
)}
{loadingCounters ? (
<Box as="span" data-component="counter" sx={counterStyles}>
<CounterLabel>{counter}</CounterLabel>
<LoadingCounter />
</Box>
)
)}
</Link>
) : (
counter !== undefined && (
<Box as="span" data-component="counter" sx={counterStyles}>
<CounterLabel>{counter}</CounterLabel>
</Box>
)
)}
</Link>
</Box>
)
},
) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps>
Expand Down
2 changes: 1 addition & 1 deletion src/deprecated/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ const LeadingVisualContainer = styled(ColoredVisualContainer)`
`

const TrailingContent = styled(ColoredVisualContainer)`
color: ${({variant, disabled}) => getItemVariant(variant, disabled).annotationColor}};
color: ${({variant, disabled}) => getItemVariant(variant, disabled).annotationColor};
margin-left: ${get('space.2')};
margin-right: 0;
width: auto;
Expand Down
2 changes: 1 addition & 1 deletion src/deprecated/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const Button = styled(ButtonBase)<ButtonBaseProps & SxProp>`
color: ${get('colors.btn.text')};
background-color: ${get('colors.btn.bg')};
border: 1px solid ${get('colors.btn.border')};
box-shadow: ${get('shadows.btn.shadow')}, ${get('shadows.btn.insetShadow')}};
box-shadow: ${get('shadows.btn.shadow')}, ${get('shadows.btn.insetShadow')};
&:hover {
background-color: ${get('colors.btn.hoverBg')};
Expand Down

0 comments on commit 674274b

Please sign in to comment.