From 01c3167e2471c61f4cad825aebe1713bdc94b78a Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Wed, 23 Jun 2021 18:46:27 -0400 Subject: [PATCH 1/2] chore: Set test directory via config rather than flag (#1319) --- jest.config.js | 3 ++- package.json | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index f03f9fcb3db..119f24e2909 100644 --- a/jest.config.js +++ b/jest.config.js @@ -4,5 +4,6 @@ module.exports = { cacheDirectory: '.test', collectCoverage: true, collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}', '!src/stories/**'], - setupFilesAfterEnv: ['/src/utils/test-matchers.tsx', '/src/utils/test-deprecations.tsx'] + setupFilesAfterEnv: ['/src/utils/test-matchers.tsx', '/src/utils/test-deprecations.tsx'], + testRegex: '/src/__tests__/.*\\.[jt]sx?$' } diff --git a/package.json b/package.json index f315ba72995..6aa720e72bf 100644 --- a/package.json +++ b/package.json @@ -17,8 +17,7 @@ "copy-esm-types": "copyfiles -u 1 \"./lib/**/*.d.ts\" ./lib-esm", "lint": "eslint '**/*.{js,ts,tsx}'", "lint:fix": "eslint '**/*.{js,ts,tsx}' --fix", - "test": "jest -- src", - "update-snapshots": "jest -u -- src", + "test": "jest", "watch": "jest --watch --no-coverage", "playroom:start": "playroom start", "playroom:build": "playroom build", From a8f3ca6dbff5c2619e067ad5118fcb784f8525bc Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Thu, 24 Jun 2021 12:52:03 -0700 Subject: [PATCH 2/2] feat(useFocusZone): update active-descendant on mousemove (#1308) --- .changeset/funny-birds-allow.md | 5 ++ src/ActionList/Item.tsx | 39 ++++++--- src/ActionList/List.tsx | 14 +++- src/FilteredActionList/FilteredActionList.tsx | 32 +++----- .../__snapshots__/ActionList.tsx.snap | 6 ++ .../__snapshots__/ActionMenu.tsx.snap | 4 +- .../__snapshots__/AnchoredOverlay.tsx.snap | 4 +- .../__snapshots__/DropdownMenu.tsx.snap | 4 +- .../__snapshots__/SelectPanel.tsx.snap | 4 +- src/__tests__/behaviors/focusZone.tsx | 45 +++++++--- src/behaviors/focusZone.ts | 82 +++++++++++++++---- 11 files changed, 172 insertions(+), 67 deletions(-) create mode 100644 .changeset/funny-birds-allow.md diff --git a/.changeset/funny-birds-allow.md b/.changeset/funny-birds-allow.md new file mode 100644 index 00000000000..edc31f10fed --- /dev/null +++ b/.changeset/funny-birds-allow.md @@ -0,0 +1,5 @@ +--- +"@primer/components": patch +--- + +Focus zones will now update active-descendant on `mousemove` over focusable elements. ActionList has been updated to handle direct (key press) vs indirect (`mousemove`, DOM change, etc.) changes to active-descendant, and will use a distinct background color for the directly activated items. diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 46d9dd521c4..cfc2839c430 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -8,7 +8,11 @@ import styled from 'styled-components' import {StyledHeader} from './Header' import {StyledDivider} from './Divider' import {useColorSchemeVar, useTheme} from '../ThemeProvider' -import {uniqueId} from '../utils/uniqueId' +import { + activeDescendantActivatedDirectly, + activeDescendantActivatedIndirectly, + isActiveDescendantAttribute +} from '../behaviors/focusZone' /** * These colors are not yet in our default theme. Need to remove this once they are added. @@ -121,8 +125,6 @@ export interface ItemProps extends Omit, ' id?: number | string } -export const itemActiveDescendantClass = `${uniqueId()}active-descendant` - const getItemVariant = (variant = 'default', disabled?: boolean) => { if (disabled) { return { @@ -176,10 +178,13 @@ const StyledItem = styled.div< display: flex; border-radius: ${get('radii.2')}; color: ${({variant, item}) => getItemVariant(variant, item?.disabled).color}; + // 2 frames on a 60hz monitor + transition: background 33.333ms linear; @media (hover: hover) and (pointer: fine) { :hover { - background: ${({hoverBackground}) => hoverBackground}; + // allow override in case another item in the list is active/focused + background: var(--item-hover-bg-override, ${({hoverBackground}) => hoverBackground}); cursor: ${({variant, item}) => getItemVariant(variant, item?.disabled).hoverCursor}; } } @@ -205,27 +210,41 @@ const StyledItem = styled.div< &:hover ${StyledItemContent}::before, // - below Hovered // '*' instead of '&' because '&' maps to separate class names depending on 'variant' - :hover + * ${StyledItemContent}::before, + :hover + * ${StyledItemContent}::before { + // allow override in case another item in the list is active/focused + border-color: var(--item-hover-divider-border-color-override, transparent) !important; + } + // - above Focused &:focus ${StyledItemContent}::before, // - below Focused // '*' instead of '&' because '&' maps to separate class names depending on 'variant' :focus + * ${StyledItemContent}::before, // - above Active Descendent - &.${itemActiveDescendantClass} ${StyledItemContent}::before, + &[${isActiveDescendantAttribute}] ${StyledItemContent}::before, // - below Active Descendent - .${itemActiveDescendantClass} + & ${StyledItemContent}::before { + [${isActiveDescendantAttribute}] + & ${StyledItemContent}::before { // '!important' because all the ':not's above give higher specificity border-color: transparent !important; } - // Focused OR Active Descendant - &:focus, - &.${itemActiveDescendantClass} { + // Active Descendant + &[${isActiveDescendantAttribute}='${activeDescendantActivatedDirectly}'] { + background: ${({focusBackground}) => focusBackground}; + } + &[${isActiveDescendantAttribute}='${activeDescendantActivatedIndirectly}'] { + background: ${({hoverBackground}) => hoverBackground}; + } + + &:focus { background: ${({focusBackground}) => focusBackground}; outline: none; } + &:active { + background: ${({focusBackground}) => focusBackground}; + } + ${sx} ` diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index b9ddc67408a..d785cfe4dd4 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -6,6 +6,7 @@ import {Divider} from './Divider' import styled from 'styled-components' import {get} from '../constants' import {SystemCssProperties} from '@styled-system/css' +import {hasActiveDescendantAttribute} from '../behaviors/focusZone' export type ItemInput = ItemProps | (Partial & {renderItem: typeof Item}) @@ -102,6 +103,11 @@ const StyledList = styled.div` * hardcoded '20px' */ line-height: 20px; + + &[${hasActiveDescendantAttribute}], &:focus-within { + --item-hover-bg-override: none; + --item-hover-divider-border-color-override: ${get('colors.selectMenu.borderSecondary')}; + } ` /** @@ -132,7 +138,7 @@ function useListVariant(variant: ListProps['variant'] = 'inset'): { /** * Lists `Item`s, either grouped or ungrouped, with a `Divider` between each `Group`. */ -export function List(props: ListProps): JSX.Element { +export const List = React.forwardRef((props, forwardedRef): JSX.Element => { // Get `sx` prop values for `List` children matching the given `List` style variation. const {firstGroupStyle, lastGroupStyle, headerStyle, itemStyle} = useListVariant(props.variant) @@ -216,7 +222,7 @@ export function List(props: ListProps): JSX.Element { } return ( - + {groups.map(({header, ...groupProps}, index) => { const hasFilledHeader = header?.variant === 'filled' const shouldShowDivider = index > 0 && !hasFilledHeader @@ -242,4 +248,6 @@ export function List(props: ListProps): JSX.Element { })} ) -} +}) + +List.displayName = 'ActionList' diff --git a/src/FilteredActionList/FilteredActionList.tsx b/src/FilteredActionList/FilteredActionList.tsx index ca471baaffe..9a5c98d8c5a 100644 --- a/src/FilteredActionList/FilteredActionList.tsx +++ b/src/FilteredActionList/FilteredActionList.tsx @@ -7,7 +7,6 @@ import {ActionList} from '../ActionList' import Spinner from '../Spinner' import {useFocusZone} from '../hooks/useFocusZone' import {uniqueId} from '../utils/uniqueId' -import {itemActiveDescendantClass} from '../ActionList/Item' import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate' import styled from 'styled-components' import {get} from '../constants' @@ -71,6 +70,7 @@ export function FilteredActionList({ [onFilterChange, setInternalFilterValue] ) + const scrollContainerRef = useRef(null) const listContainerRef = useRef(null) const inputRef = useProvidedRefOrCreate(providedInputRef) const activeDescendantRef = useRef() @@ -93,38 +93,26 @@ export function FilteredActionList({ containerRef: listContainerRef, focusOutBehavior: 'wrap', focusableElementFilter: element => { - if (element instanceof HTMLInputElement) { - // No active-descendant focus on checkboxes in list items - return false - } - return true + return !(element instanceof HTMLInputElement) }, activeDescendantFocus: inputRef, - onActiveDescendantChanged: (current, previous) => { + onActiveDescendantChanged: (current, previous, directlyActivated) => { activeDescendantRef.current = current - if (previous) { - previous.classList.remove(itemActiveDescendantClass) - } - - if (current) { - current.classList.add(itemActiveDescendantClass) - - if (listContainerRef.current) { - scrollIntoViewingArea(current, listContainerRef.current) - } + if (current && scrollContainerRef.current && directlyActivated) { + scrollIntoViewingArea(current, scrollContainerRef.current) } } }) useEffect(() => { // if items changed, we want to instantly move active descendant into view - if (activeDescendantRef.current && listContainerRef.current) { - scrollIntoViewingArea(activeDescendantRef.current, listContainerRef.current, undefined, 'auto') + if (activeDescendantRef.current && scrollContainerRef.current) { + scrollIntoViewingArea(activeDescendantRef.current, scrollContainerRef.current, undefined, 'auto') } }, [items]) - useScrollFlash(listContainerRef) + useScrollFlash(scrollContainerRef) return ( @@ -143,13 +131,13 @@ export function FilteredActionList({ {...textInputProps} /> - + {loading ? ( ) : ( - + )} diff --git a/src/__tests__/__snapshots__/ActionList.tsx.snap b/src/__tests__/__snapshots__/ActionList.tsx.snap index 5b561ba1bc7..5d1308e0c23 100644 --- a/src/__tests__/__snapshots__/ActionList.tsx.snap +++ b/src/__tests__/__snapshots__/ActionList.tsx.snap @@ -11,6 +11,12 @@ exports[`ActionList renders consistently 1`] = ` line-height: 20px; } +.c0[data-has-active-descendant], +.c0:focus-within { + --item-hover-bg-override: none; + --item-hover-divider-border-color-override: hsla(214,13%,93%,1); +} +
diff --git a/src/__tests__/__snapshots__/ActionMenu.tsx.snap b/src/__tests__/__snapshots__/ActionMenu.tsx.snap index 7db066bfa39..a92e9626f63 100644 --- a/src/__tests__/__snapshots__/ActionMenu.tsx.snap +++ b/src/__tests__/__snapshots__/ActionMenu.tsx.snap @@ -70,9 +70,9 @@ exports[`ActionMenu renders consistently 1`] = `