-
Notifications
You must be signed in to change notification settings - Fork 638
Breadcrumbs: Remove sx
from component
#6844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
4bb272f
Remove `sx` from `Breadcrumbs`
TylerJDev 2375626
Remove PolymorphicForwardRefComponent
TylerJDev 4d83e1b
Merge branch 'main' into remove-sx-from-breadcrumbs
TylerJDev b2e1ac2
Remove from `styled-react`
TylerJDev 5be4f2e
Merge branch 'main' into remove-sx-from-breadcrumbs
TylerJDev b13d97d
Run format
TylerJDev 8f2a9f3
Remove css for story
TylerJDev a514560
Add changeset
TylerJDev adfc912
Merge branch 'main' of github.com:primer/react into remove-sx-from-br…
francinelucca ece417f
re-add tests
francinelucca 3ec524c
add breadcrumbs to styled-react
francinelucca d12d8ff
fix export
francinelucca 98a37c3
modify test
francinelucca ee2e4bb
fix types.
francinelucca a6196d0
fix types
francinelucca 338d1a2
Merge branch 'main' of github.com:primer/react into remove-sx-from-br…
francinelucca 029cd7a
eslint fix
francinelucca 0d08da1
remove wrapping
francinelucca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@primer/react': major | ||
--- | ||
|
||
Update `Breadcrumbs` to no longer support sx |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,7 @@ | ||
import {clsx} from 'clsx' | ||
import type {To} from 'history' | ||
import React, {useState, useRef, useCallback, useEffect, useMemo} from 'react' | ||
import type {SxProp} from '../sx' | ||
import type {ComponentProps} from '../utils/types' | ||
import React, {useState, useRef, useCallback, useEffect, useMemo, type ForwardedRef} from 'react' | ||
import classes from './Breadcrumbs.module.css' | ||
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' | ||
import {BoxWithFallback} from '../internal/components/BoxWithFallback' | ||
import Details from '../Details' | ||
import {ActionList} from '../ActionList' | ||
import {IconButton} from '../Button/IconButton' | ||
|
@@ -16,27 +12,29 @@ import {useOnEscapePress} from '../hooks/useOnEscapePress' | |
import {useOnOutsideClick} from '../hooks/useOnOutsideClick' | ||
import {useFeatureFlag} from '../FeatureFlags' | ||
|
||
export type BreadcrumbsProps = React.PropsWithChildren< | ||
{ | ||
/** | ||
* Optional class name for the breadcrumbs container. | ||
*/ | ||
className?: string | ||
/** | ||
* Controls the overflow behavior of the breadcrumbs. | ||
* By default all overflowing crumbs will "wrap" in the given space taking up extra height. | ||
* In the "menu" option we'll see the overflowing crumbs as part of a menu like dropdown instead of the root breadcrumb. | ||
* In "menu-with-root" we see that instead of the root, the menu button will take the place of the next breadcrumb. | ||
*/ | ||
overflow?: 'wrap' | 'menu' | 'menu-with-root' | ||
/** | ||
* Controls the visual variant of the breadcrumbs. | ||
* By default, the breadcrumbs will have a normal appearance. | ||
* In the "spacious" option, the breadcrumbs will have increased padding and a more relaxed layout. | ||
*/ | ||
variant?: 'normal' | 'spacious' | ||
} & SxProp | ||
> | ||
export type BreadcrumbsProps = React.PropsWithChildren<{ | ||
/** | ||
* Optional class name for the breadcrumbs container. | ||
*/ | ||
className?: string | ||
/** | ||
* Controls the overflow behavior of the breadcrumbs. | ||
* By default all overflowing crumbs will "wrap" in the given space taking up extra height. | ||
* In the "menu" option we'll see the overflowing crumbs as part of a menu like dropdown instead of the root breadcrumb. | ||
* In "menu-with-root" we see that instead of the root, the menu button will take the place of the next breadcrumb. | ||
*/ | ||
overflow?: 'wrap' | 'menu' | 'menu-with-root' | ||
/** | ||
* Controls the visual variant of the breadcrumbs. | ||
* By default, the breadcrumbs will have a normal appearance. | ||
* In the "spacious" option, the breadcrumbs will have increased padding and a more relaxed layout. | ||
*/ | ||
variant?: 'normal' | 'spacious' | ||
/** | ||
* Allows passing of CSS custom properties to the breadcrumbs container. | ||
*/ | ||
style?: React.CSSProperties | ||
}> | ||
|
||
const BreadcrumbsList = ({children}: React.PropsWithChildren) => { | ||
return <ol className={classes.BreadcrumbsList}>{children}</ol> | ||
|
@@ -146,7 +144,7 @@ const getValidChildren = (children: React.ReactNode) => { | |
return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] | ||
} | ||
|
||
function Breadcrumbs({className, children, sx: sxProp, overflow = 'wrap', variant = 'normal'}: BreadcrumbsProps) { | ||
function Breadcrumbs({className, children, style, overflow = 'wrap', variant = 'normal'}: BreadcrumbsProps) { | ||
const overflowMenuEnabled = useFeatureFlag('primer_react_breadcrumbs_overflow_menu') | ||
const wrappedChildren = React.Children.map(children, child => <li className={classes.ItemWrapper}>{child}</li>) | ||
const containerRef = useRef<HTMLElement>(null) | ||
|
@@ -176,10 +174,6 @@ function Breadcrumbs({className, children, sx: sxProp, overflow = 'wrap', varian | |
const MENU_BUTTON_FALLBACK_WIDTH = 32 // Design system small IconButton | ||
const [menuButtonWidth, setMenuButtonWidth] = useState(MENU_BUTTON_FALLBACK_WIDTH) | ||
|
||
// if (typeof window !== 'undefined') { | ||
// effectiveOverflow = overflow | ||
// } | ||
|
||
useEffect(() => { | ||
const listElement = containerRef.current?.querySelector('ol') | ||
if ( | ||
francinelucca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -333,27 +327,25 @@ function Breadcrumbs({className, children, sx: sxProp, overflow = 'wrap', varian | |
}, [overflowMenuEnabled, overflow, menuItems, effectiveHideRoot, measureMenuButton, visibleItems, rootItem, children]) | ||
|
||
return overflowMenuEnabled ? ( | ||
<BoxWithFallback | ||
as="nav" | ||
<nav | ||
className={clsx(className, classes.BreadcrumbsBase)} | ||
aria-label="Breadcrumbs" | ||
sx={sxProp} | ||
style={style} | ||
ref={containerRef} | ||
data-overflow={overflow} | ||
data-variant={variant} | ||
> | ||
<BreadcrumbsList>{finalChildren}</BreadcrumbsList> | ||
</BoxWithFallback> | ||
</nav> | ||
) : ( | ||
<BoxWithFallback | ||
as="nav" | ||
<nav | ||
className={clsx(className, classes.BreadcrumbsBase)} | ||
aria-label="Breadcrumbs" | ||
sx={sxProp} | ||
style={style} | ||
data-variant={variant} | ||
> | ||
<BreadcrumbsList>{wrappedChildren}</BreadcrumbsList> | ||
</BoxWithFallback> | ||
</nav> | ||
) | ||
} | ||
|
||
|
@@ -367,31 +359,40 @@ const ItemSeparator = () => { | |
) | ||
} | ||
|
||
type StyledBreadcrumbsItemProps = { | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
type DistributiveOmit<T, TOmitted extends PropertyKey> = T extends any ? Omit<T, TOmitted> : never | ||
|
||
type StyledBreadcrumbsItemProps<As extends React.ElementType> = { | ||
as?: As | ||
to?: To | ||
selected?: boolean | ||
className?: string | ||
} & SxProp & | ||
React.HTMLAttributes<HTMLAnchorElement> & | ||
React.ComponentPropsWithRef<'a'> | ||
|
||
const BreadcrumbsItem = React.forwardRef(({selected, className, ...rest}, ref) => { | ||
style?: React.CSSProperties | ||
} & DistributiveOmit<React.ComponentPropsWithRef<React.ElementType extends As ? 'a' : As>, 'as'> | ||
|
||
function BreadcrumbsItemComponent<As extends React.ElementType>( | ||
props: StyledBreadcrumbsItemProps<As>, | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
ref: ForwardedRef<any>, | ||
) { | ||
const {as: Component = 'a', selected, className, ...rest} = props | ||
return ( | ||
<BoxWithFallback | ||
as="a" | ||
<Component | ||
className={clsx(className, classes.Item, selected && 'selected')} | ||
aria-current={selected ? 'page' : undefined} | ||
ref={ref} | ||
{...rest} | ||
/> | ||
) | ||
}) as PolymorphicForwardRefComponent<'a', StyledBreadcrumbsItemProps> | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
Breadcrumbs.displayName = 'Breadcrumbs' | ||
BreadcrumbsItemComponent.displayName = 'Breadcrumbs.Item' | ||
|
||
BreadcrumbsItem.displayName = 'Breadcrumbs.Item' | ||
const BreadcrumbsItem = React.forwardRef(BreadcrumbsItemComponent) | ||
|
||
Breadcrumbs.displayName = 'Breadcrumbs' | ||
|
||
export type BreadcrumbsItemProps = ComponentProps<typeof BreadcrumbsItem> | ||
export type BreadcrumbsItemProps<As extends React.ElementType = 'a'> = StyledBreadcrumbsItemProps<As> | ||
export default Object.assign(Breadcrumbs, {Item: BreadcrumbsItem}) | ||
|
||
/** | ||
|
@@ -407,4 +408,4 @@ export type BreadcrumbProps = BreadcrumbsProps | |
/** | ||
* @deprecated Use the `BreadcrumbsItemProps` type instead | ||
*/ | ||
export type BreadcrumbItemProps = ComponentProps<typeof BreadcrumbsItem> | ||
export type BreadcrumbItemProps<As extends React.ElementType = 'a'> = BreadcrumbsItemProps<As> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import {Breadcrumbs as PrimerBreadcrumbs} from '@primer/react' | ||
import type { | ||
BreadcrumbsProps as PrimerBreadcrumbsProps, | ||
BreadcrumbsItemProps as PrimerBreadcrumbsItemsProps, | ||
} from '@primer/react' | ||
import {sx, type SxProp} from '../sx' | ||
import styled from 'styled-components' | ||
import {type ForwardRefComponent} from '../polymorphic' | ||
import type React from 'react' | ||
|
||
type BreadcrumbsProps = PrimerBreadcrumbsProps & SxProp | ||
type BreadcrumbsItemProps<As extends React.ElementType = 'a'> = PrimerBreadcrumbsItemsProps<As> & SxProp | ||
|
||
const BreadcrumbsImpl = styled(PrimerBreadcrumbs).withConfig({ | ||
shouldForwardProp: prop => (prop as keyof BreadcrumbsProps) !== 'sx', | ||
})<BreadcrumbsProps>` | ||
${sx} | ||
` | ||
|
||
const StyledBreadcrumbsItem: ForwardRefComponent<'a', BreadcrumbsItemProps> = styled(PrimerBreadcrumbs.Item).withConfig( | ||
{ | ||
shouldForwardProp: prop => (prop as keyof BreadcrumbsItemProps) !== 'sx', | ||
}, | ||
)<BreadcrumbsItemProps>` | ||
${sx} | ||
` | ||
|
||
const BreadcrumbsItem = ({as, ...props}: BreadcrumbsItemProps) => ( | ||
<StyledBreadcrumbsItem {...props} {...(as ? {forwardedAs: as} : {})} /> | ||
) | ||
|
||
const Breadcrumbs: ForwardRefComponent<'nav', BreadcrumbsProps> & {Item: typeof BreadcrumbsItem} = Object.assign( | ||
BreadcrumbsImpl, | ||
{Item: BreadcrumbsItem}, | ||
) | ||
|
||
/** | ||
* @deprecated Use the `Breadcrumbs` component instead (i.e. `<Breadcrumb>` → `<Breadcrumbs>`) | ||
*/ | ||
const Breadcrumb = Breadcrumbs | ||
|
||
export {Breadcrumbs, Breadcrumb, type BreadcrumbsProps, type BreadcrumbsItemProps} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.