Skip to content
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

ActionList Fixes #1215

Merged
merged 18 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/blue-items-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Hide divider before `ActionList.Group`s with filled header
5 changes: 5 additions & 0 deletions .changeset/hip-bugs-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Align `Item` description to when rendered in-line
5 changes: 5 additions & 0 deletions .changeset/long-eagles-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Allow `focusZoneSettings` to be passed into `AnchoredOverlay`
5 changes: 5 additions & 0 deletions .changeset/lovely-poems-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Add `selectionVariant: 'multiple'` for `Item`s. These will use a checkbox input instead of a checkmark icon for selected state
36 changes: 33 additions & 3 deletions src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ export interface ItemProps extends Omit<React.ComponentPropsWithoutRef<'div'>, '
*/
selected?: boolean

/**
* For `Item`s which can be selected, whether `multiple` `Item`s or a `single` `Item` can be selected
*/
selectionVariant?: 'single' | 'multiple'

/**
* Designates the group that an item belongs to.
*/
Expand Down Expand Up @@ -129,6 +134,7 @@ const StyledItem = styled.div<{variant: ItemProps['variant']; item?: ItemInput}
`

const StyledTextContainer = styled.div<{descriptionVariant: ItemProps['descriptionVariant']}>`
display: flex;
flex-direction: ${({descriptionVariant}) => (descriptionVariant === 'inline' ? 'row' : 'column')};
`

Expand Down Expand Up @@ -163,8 +169,9 @@ const TrailingVisualContainer = styled(BaseVisualContainer)`
justify-content: flex-end;
`

const DescriptionContainer = styled.span`
const DescriptionContainer = styled.span<{descriptionVariant: ItemProps['descriptionVariant']}>`
color: ${get('colors.text.secondary')};
margin-left: ${({descriptionVariant}) => (descriptionVariant === 'inline' ? get('space.2') : 0)};
`

/**
Expand All @@ -176,6 +183,7 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
description,
descriptionVariant = 'inline',
selected,
selectionVariant,
leadingVisual: LeadingVisual,
trailingIcon: TrailingIcon,
trailingText,
Expand All @@ -195,6 +203,12 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
return
}
onKeyPress?.(event)
const isCheckbox = event.target instanceof HTMLInputElement && event.target.type === 'checkbox'
if (isCheckbox && event.key === ' ') {
// space key on a checkbox will also trigger a click event. Ignore the space key so we don't get double events
return
}

if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) {
onAction?.(itemProps as ItemProps, event)
}
Expand Down Expand Up @@ -225,7 +239,21 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
onKeyPress={keyPressHandler}
onClick={clickHandler}
>
{!!selected === selected && <LeadingVisualContainer>{selected && <CheckIcon />}</LeadingVisualContainer>}
{!!selected === selected && (
<LeadingVisualContainer>
{selectionVariant === 'multiple' ? (
<>
{/*
* readOnly is required because we are doing a one-way bind to `checked`.
* aria-readonly="false" tells screen that they can still interact with the checkbox
*/}
<input type="checkbox" checked={selected} aria-label={text} readOnly aria-readonly="false" />
</>
) : (
selected && <CheckIcon />
)}
</LeadingVisualContainer>
)}
{LeadingVisual && (
<LeadingVisualContainer variant={variant} disabled={disabled}>
<LeadingVisual />
Expand All @@ -235,7 +263,9 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
{(text || description) && (
<StyledTextContainer descriptionVariant={descriptionVariant}>
{text && <div>{text}</div>}
{description && <DescriptionContainer>{description}</DescriptionContainer>}
{description && (
<DescriptionContainer descriptionVariant={descriptionVariant}>{description}</DescriptionContainer>
)}
</StyledTextContainer>
)}
{(TrailingIcon || trailingText) && (
Expand Down
62 changes: 43 additions & 19 deletions src/ActionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ export interface ListPropsBase {
* - `"full"` - `List` children are flush (vertically and horizontally) with `List` edges
*/
variant?: 'inset' | 'full'

/**
* For `Item`s which can be selected, whether `multiple` `Item`s or a `single` `Item` can be selected
*/
selectionVariant?: 'single' | 'multiple'
}

/**
Expand Down Expand Up @@ -82,6 +87,12 @@ export type ListProps = ListPropsBase | GroupedListProps

const StyledList = styled.div`
font-size: ${get('fontSizes.1')};
/* 14px font-size * 1.428571429 = 20px line height
*
* TODO: When rem-based spacing on a 4px scale lands, replace
* hardcoded '20px'
*/
line-height: 20px;
`

/**
Expand Down Expand Up @@ -138,7 +149,15 @@ export function List(props: ListProps): JSX.Element {
const renderItem = (itemProps: ItemInput, item: ItemInput) => {
const ItemComponent = ('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item
const key = itemProps.key ?? itemProps.id?.toString() ?? uniqueId()
return <ItemComponent {...itemProps} key={key} sx={{...itemStyle, ...itemProps.sx}} item={item} />
return (
<ItemComponent
selectionVariant={props.selectionVariant}
{...itemProps}
key={key}
sx={{...itemStyle, ...itemProps.sx}}
item={item}
/>
)
}

/**
Expand Down Expand Up @@ -188,24 +207,29 @@ export function List(props: ListProps): JSX.Element {

return (
<StyledList {...props}>
{groups?.map(({header, ...groupProps}, index) => (
<React.Fragment key={groupProps.groupId}>
{renderGroup({
sx: {
...(index === 0 && firstGroupStyle),
...(index === groups.length - 1 && lastGroupStyle)
},
...(header && {
header: {
...header,
sx: {...headerStyle, ...header?.sx}
}
}),
...groupProps
})}
{index + 1 !== groups.length && <Divider key={`${groupProps.groupId}-divider`} />}
</React.Fragment>
))}
{groups?.map(({header, ...groupProps}, index) => {
const hasFilledHeader = header?.variant === 'filled'
const shouldShowDivider = index > 0 && !hasFilledHeader
return (
<React.Fragment key={groupProps.groupId}>
{shouldShowDivider ? <Divider key={`${groupProps.groupId}-divider`} /> : null}
{renderGroup({
sx: {
...(index === 0 && firstGroupStyle),
...(index === groups.length - 1 && lastGroupStyle),
...(index > 0 && !shouldShowDivider && {mt: 2})
},
...(header && {
header: {
...header,
sx: {...headerStyle, ...header?.sx}
}
}),
...groupProps
})}
</React.Fragment>
)
})}
</StyledList>
)
}
15 changes: 13 additions & 2 deletions src/AnchoredOverlay/AnchoredOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {useFocusTrap} from '../hooks/useFocusTrap'
import {useFocusZone} from '../hooks/useFocusZone'
import {useAnchoredPosition, useRenderForcingRef} from '../hooks'
import {uniqueId} from '../utils/uniqueId'
import {FocusZoneSettings} from '../behaviors/focusZone'

export interface AnchoredOverlayProps extends Pick<OverlayProps, 'height' | 'width'> {
/**
Expand Down Expand Up @@ -31,6 +32,11 @@ export interface AnchoredOverlayProps extends Pick<OverlayProps, 'height' | 'wid
* Props to be spread on the internal `Overlay` component.
*/
overlayProps?: Partial<OverlayProps>

/**
* Settings to apply to the Focus Zone on the internal `Overlay` component.
*/
focusZoneSettings?: Partial<FocusZoneSettings>
}

/**
Expand All @@ -44,8 +50,9 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
onOpen,
onClose,
height,
width,
overlayProps,
width
focusZoneSettings
}) => {
const anchorRef = useRef<HTMLElement>(null)
const [overlayRef, updateOverlayRef] = useRenderForcingRef<HTMLDivElement>()
Expand Down Expand Up @@ -85,7 +92,11 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
return position && {top: `${position.top}px`, left: `${position.left}px`}
}, [position])

useFocusZone({containerRef: overlayRef, disabled: !open || !position})
useFocusZone({
containerRef: overlayRef,
disabled: !open || !position,
...focusZoneSettings
})
useFocusTrap({containerRef: overlayRef, disabled: !open || !position})

return (
Expand Down
1 change: 0 additions & 1 deletion src/Overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & System
height: ${props => heightMap[props.height || 'auto']};
width: ${props => widthMap[props.width || 'auto']};
border-radius: 12px;
overflow: hidden;
animation: overlay-appear 200ms ${get('animation.easeOutCubic')};

@keyframes overlay-appear {
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/ActionList.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ exports[`ActionList renders consistently 1`] = `

.c0 {
font-size: 14px;
line-height: 20px;
}

<div
Expand Down
69 changes: 65 additions & 4 deletions src/stories/ActionList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ export default meta
const ErsatzOverlay = styled.div`
border-radius: 12px;
box-shadow: 0 1px 3px rgba(0, 0, 0, 0.12), 0 8px 24px rgba(149, 157, 165, 0.2);
overflow: hidden;
position: absolute;
min-width: 192px;
max-width: 640px;
`

export function ActionsStory(): JSX.Element {
Expand Down Expand Up @@ -98,7 +100,49 @@ export function SimpleListStory(): JSX.Element {
}
SimpleListStory.storyName = 'Simple List'

export function ComplexListStory(): JSX.Element {
const selectListItems = new Array(6).fill(undefined).map((_, i) => {
return {
text: `Item ${i}`,
id: i
}
})

export function SingleSelectListStory(): JSX.Element {
return (
<>
<h1>Single Select List</h1>
<ErsatzOverlay>
<ActionList
items={selectListItems.map((item, index) => ({
...item,
selected: index === 1
}))}
/>
</ErsatzOverlay>
</>
)
}
SingleSelectListStory.storyName = 'Single Select'

export function MultiSelectListStory(): JSX.Element {
return (
<>
<h1>Multi Select List</h1>
<ErsatzOverlay>
<ActionList
selectionVariant="multiple"
items={selectListItems.map((item, index) => ({
...item,
selected: index === 1 || index === 3
}))}
/>
</ErsatzOverlay>
</>
)
}
MultiSelectListStory.storyName = 'Multi Select'

export function ComplexListInsetVariantStory(): JSX.Element {
const StyledDiv = styled.div`
${sx}
`
Expand Down Expand Up @@ -132,7 +176,13 @@ export function ComplexListStory(): JSX.Element {
]}
items={[
{leadingVisual: TypographyIcon, text: 'Rename', groupId: '0'},
{leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
{
leadingVisual: VersionsIcon,
text: 'Duplicate',
description: 'Create a copy',
descriptionVariant: 'inline',
groupId: '0'
},
{
leadingVisual: SearchIcon,
text: 'repo:github/memex,github/github',
Expand Down Expand Up @@ -163,7 +213,18 @@ export function ComplexListStory(): JSX.Element {
]}
/>
</ErsatzOverlay>
</>
)
}
ComplexListInsetVariantStory.storyName = 'Complex List — Inset Variant'

export function ComplexListFullVariantStory(): JSX.Element {
const StyledDiv = styled.div`
${sx}
`
return (
<>
<h1>Complex List</h1>
<h2>Full Variant</h2>
<ErsatzOverlay>
<ActionList
Expand Down Expand Up @@ -226,7 +287,7 @@ export function ComplexListStory(): JSX.Element {
</>
)
}
ComplexListStory.storyName = 'Complex List'
ComplexListFullVariantStory.storyName = 'Complex List — Full Variant'

export function HeaderStory(): JSX.Element {
return (
Expand Down