Skip to content

Commit

Permalink
SelectPanel: Update SelectPanel to use modern ActionList (#4794)
Browse files Browse the repository at this point in the history
* super wip

* just use the actionlist component and revert the type updates

* Update packages/react/src/FilteredActionList/FilteredActionList.tsx

* Some more progress

* revert the type changes and cast it :/

* clean

* wip

* wip wip

* add stories

* fix linting

* add tests for groups

* Map groups

* Update story names for e2e tests

* oops remove unintended file

* update story name

* same - update story name

* disable animations

* test(vrt): update snapshots

* Update tests since new action list has different semantics for group headings

* logging

* pass the rest

* extract children and use before text

* remove logging

* test(vrt): update snapshots

* add active styles to ActionList

* rename component name to be clearer

* remove variant full from examples

* tiny clean up

* fix showItemDividers

* another tiny cleanup

* pull MappedActionListItem to make it stable

* test(vrt): update snapshots

* show active styles only when used with keyboard

* backward compat: expose id as data-id

* update snapshots

* add story for long strings

* fishing for errors

* backward compatibility for renderItem

* remove todo now

* add a feature flag

* clean up dual filter list setup

* run jests test with both states of feature flags

* refactor snapshot tests with scenarios

* remove feature flag for main

* test(vrt): update snapshots

* add feature flag to e2e matrix

* test(vrt): update snapshots

* backward compat: allow groupMetadata to be empty array

* sigh newline

* Create sour-cooks-dress.md

* copy SelectPanel snapshots from main

* remove unrelated changes in this PR

* test(vrt): update snapshots

* set active styles for both active-descendant types

* test(vrt): update snapshots

---------

Co-authored-by: broccolinisoup <broccolinisoup@users.noreply.github.com>
Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: siddharthkp <siddharthkp@users.noreply.github.com>
  • Loading branch information
4 people committed Sep 10, 2024
1 parent c282642 commit 5f996c6
Show file tree
Hide file tree
Showing 130 changed files with 1,016 additions and 445 deletions.
5 changes: 5 additions & 0 deletions .changeset/sour-cooks-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

SelectPanel: Update SelectPanel to use modern ActionList behind a feature flag `primer_react_select_panel_with_modern_action_list`
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 6 additions & 3 deletions e2e/components/SelectPanel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {matrix} from '../test-helpers/matrix'

const scenarios = matrix({
theme: themes,
modernActionList: [false, true],
story: [
{id: 'components-selectpanel--default', name: 'Default'},
{id: 'components-selectpanel-features--single-select', name: 'Single Select'},
Expand Down Expand Up @@ -38,21 +39,23 @@ test.describe('SelectPanel', () => {
for (const scenario of scenarios) {
const name = scenario.story.name
const theme = scenario.theme
const flag = scenario.modernActionList ? `.modern-action-list--${scenario.modernActionList}` : ''

const globals = {
colorScheme: scenario.theme,
featureFlags: {primer_react_select_panel_with_modern_action_list: scenario.modernActionList},
}

test(`${name} @vrt ${theme}`, async ({page}) => {
test(`${name} @vrt ${theme} ${flag}`, async ({page}) => {
await visit(page, {id: scenario.story.id, globals})

// Open select panel
await page.keyboard.press('Tab')
await page.keyboard.press('Enter')
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(`SelectPanel.${name}.${theme}.png`)
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(`SelectPanel.${name}.${theme}${flag}.png`)
})

test(`${name} axe @aat ${theme}`, async ({page}) => {
test(`${name} axe @aat ${theme} ${flag}`, async ({page}) => {
await visit(page, {id: scenario.story.id, globals})
await expect(page).toHaveNoViolations()
})
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
'&:hover:not([aria-disabled]):not([data-inactive]):not([data-loading]) + &, &[data-focus-visible-added] + li': {
'--divider-color': 'transparent',
},
...(active ? activeStyles : {}),

/** Active styles */
...(active ? activeStyles : {}), // NavList
'&[data-is-active-descendant]': activeStyles, // SelectPanel

...(!buttonSemantics ? hoverStyles : {}),
}

Expand Down
1 change: 1 addition & 0 deletions packages/react/src/FeatureFlags/DefaultFeatureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({
primer_react_css_modules_staff: false,
primer_react_css_modules_ga: false,
primer_react_action_list_item_as_button: false,
primer_react_select_panel_with_modern_action_list: false,
})
16 changes: 16 additions & 0 deletions packages/react/src/FilteredActionList/FilteredActionListEntry.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react'
import type {FilteredActionListProps} from './FilteredActionListWithDeprecatedActionList'
import {FilteredActionList as WithDeprecatedActionList} from './FilteredActionListWithDeprecatedActionList'
import {FilteredActionList as WithStableActionList} from './FilteredActionListWithModernActionList'
import {useFeatureFlag} from '../FeatureFlags'

export function FilteredActionList(props: FilteredActionListProps): JSX.Element {
const enabled = useFeatureFlag('primer_react_select_panel_with_modern_action_list')

if (enabled) return <WithStableActionList {...props} />
else return <WithDeprecatedActionList {...props} />
}

FilteredActionList.displayName = 'FilteredActionList'

export type {FilteredActionListProps}
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {scrollIntoView} from '@primer/behaviors'
import type {KeyboardEventHandler} from 'react'
import React, {useCallback, useEffect, useRef} from 'react'
import styled from 'styled-components'
import Box from '../Box'
import Spinner from '../Spinner'
import type {TextInputProps} from '../TextInput'
import TextInput from '../TextInput'
import {get} from '../constants'
import {ActionList} from '../ActionList'
import type {GroupedListProps, ListPropsBase, ItemInput} from '../SelectPanel/types'
import {useFocusZone} from '../hooks/useFocusZone'
import {useId} from '../hooks/useId'
import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
import useScrollFlash from '../hooks/useScrollFlash'
import {VisuallyHidden} from '../internal/components/VisuallyHidden'
import type {SxProp} from '../sx'

import {isValidElementType} from 'react-is'
import type {RenderItemFn} from '../deprecated/ActionList/List'

const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}

export interface FilteredActionListProps
extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
ListPropsBase,
SxProp {
loading?: boolean
placeholderText?: string
filterValue?: string
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
inputRef?: React.RefObject<HTMLInputElement>
}

const StyledHeader = styled.div`
box-shadow: 0 1px 0 ${get('colors.border.default')};
z-index: 1;
`

export function FilteredActionList({
loading = false,
placeholderText,
filterValue: externalFilterValue,
onFilterChange,
items,
textInputProps,
inputRef: providedInputRef,
sx,
groupMetadata,
showItemDividers,
...listProps
}: FilteredActionListProps): JSX.Element {
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
const onInputChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.target.value
onFilterChange(value, e)
setInternalFilterValue(value)
},
[onFilterChange, setInternalFilterValue],
)

const scrollContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLUListElement>(null)
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
const activeDescendantRef = useRef<HTMLElement>()
const listId = useId()
const inputDescriptionTextId = useId()
const onInputKeyPress: KeyboardEventHandler = useCallback(
event => {
if (event.key === 'Enter' && activeDescendantRef.current) {
event.preventDefault()
event.nativeEvent.stopImmediatePropagation()

// Forward Enter key press to active descendant so that item gets activated
const activeDescendantEvent = new KeyboardEvent(event.type, event.nativeEvent)
activeDescendantRef.current.dispatchEvent(activeDescendantEvent)
}
},
[activeDescendantRef],
)

useFocusZone(
{
containerRef: listContainerRef,
focusOutBehavior: 'wrap',
focusableElementFilter: element => {
return !(element instanceof HTMLInputElement)
},
activeDescendantFocus: inputRef,
onActiveDescendantChanged: (current, previous, directlyActivated) => {
activeDescendantRef.current = current

if (current && scrollContainerRef.current && directlyActivated) {
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
}
},
},
[
// List ref isn't set while loading. Need to re-bind focus zone when it changes
loading,
],
)

useEffect(() => {
// if items changed, we want to instantly move active descendant into view
if (activeDescendantRef.current && scrollContainerRef.current) {
scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {...menuScrollMargins, behavior: 'auto'})
}
}, [items])

useScrollFlash(scrollContainerRef)

function getItemListForEachGroup(groupId: string) {
const itemsInGroup = []
for (const item of items) {
// Look up the group associated with the current item.
if (item.groupId === groupId) {
itemsInGroup.push(item)
}
}
return itemsInGroup
}

return (
<Box display="flex" flexDirection="column" overflow="hidden" sx={sx}>
<StyledHeader>
<TextInput
ref={inputRef}
block
width="auto"
color="fg.default"
value={filterValue}
onChange={onInputChange}
onKeyPress={onInputKeyPress}
placeholder={placeholderText}
aria-label={placeholderText}
aria-controls={listId}
aria-describedby={inputDescriptionTextId}
{...textInputProps}
/>
</StyledHeader>
<VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
<Box ref={scrollContainerRef} overflow="auto">
{loading ? (
<Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
<Spinner />
</Box>
) : (
<ActionList ref={listContainerRef} showDividers={showItemDividers} {...listProps} role="listbox" id={listId}>
{groupMetadata?.length
? groupMetadata.map((group, index) => {
return (
<ActionList.Group key={index}>
<ActionList.GroupHeading variant={group.header?.variant ? group.header.variant : undefined}>
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
</ActionList.GroupHeading>
{getItemListForEachGroup(group.groupId).map((item, index) => {
return <MappedActionListItem key={index} {...item} renderItem={listProps.renderItem} />
})}
</ActionList.Group>
)
})
: items.map((item, index) => {
return <MappedActionListItem key={index} {...item} renderItem={listProps.renderItem} />
})}
</ActionList>
)}
</Box>
</Box>
)
}

function MappedActionListItem(item: ItemInput & {renderItem?: RenderItemFn}) {
// keep backward compatibility for renderItem
// escape hatch for custom Item rendering
if (typeof item.renderItem === 'function') return item.renderItem(item)

const {
id,
description,
descriptionVariant,
text,
trailingVisual: TrailingVisual,
leadingVisual: LeadingVisual,
trailingText,
trailingIcon: TrailingIcon,
onAction,
children,
...rest
} = item

return (
<ActionList.Item
role="option"
// @ts-ignore - for now
onSelect={(e: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>) => {
if (typeof onAction === 'function')
onAction(item, e as React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>)
}}
data-id={id}
{...rest}
>
{LeadingVisual ? (
<ActionList.LeadingVisual>
<LeadingVisual />
</ActionList.LeadingVisual>
) : null}
{children}
{text}
{description ? <ActionList.Description variant={descriptionVariant}>{description}</ActionList.Description> : null}
{TrailingVisual ? (
<ActionList.TrailingVisual>
{typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? (
<TrailingVisual />
) : (
TrailingVisual
)}
</ActionList.TrailingVisual>
) : TrailingIcon || trailingText ? (
<ActionList.TrailingVisual>
{trailingText}
{TrailingIcon && <TrailingIcon />}
</ActionList.TrailingVisual>
) : null}
</ActionList.Item>
)
}

FilteredActionList.displayName = 'FilteredActionList'
4 changes: 2 additions & 2 deletions packages/react/src/FilteredActionList/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export {FilteredActionList} from './FilteredActionList'
export type {FilteredActionListProps} from './FilteredActionList'
export {FilteredActionList} from './FilteredActionListEntry'
export type {FilteredActionListProps} from './FilteredActionListEntry'
Loading

0 comments on commit 5f996c6

Please sign in to comment.