-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: support sections and headers in RAC gridlist #8667
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
base: main
Are you sure you want to change the base?
Changes from all commits
13f30a4
42939bf
1cec0c1
72c57d3
56c400e
519e0c6
fe146d3
5e6236d
7aea99e
88a387d
6bcdd2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
/* | ||
* Copyright 2020 Adobe. All rights reserved. | ||
* This file is licensed to you under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. You may obtain a copy | ||
* of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software distributed under | ||
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS | ||
* OF ANY KIND, either express or implied. See the License for the specific language | ||
* governing permissions and limitations under the License. | ||
*/ | ||
|
||
import {DOMAttributes, RefObject} from '@react-types/shared'; | ||
import type {ListState} from '@react-stately/list'; | ||
import {useLabels, useSlotId} from '@react-aria/utils'; | ||
|
||
export interface AriaGridListSectionProps { | ||
/** An accessibility label for the section. Required if `heading` is not present. */ | ||
'aria-label'?: string | ||
} | ||
|
||
export interface GridListSectionAria { | ||
/** Props for the wrapper list item. */ | ||
rowProps: DOMAttributes, | ||
|
||
/** Props for the heading element, if any. */ | ||
rowHeaderProps: DOMAttributes, | ||
|
||
/** Props for the grid's row group element. */ | ||
rowGroupProps: DOMAttributes | ||
} | ||
|
||
/** | ||
* Provides the behavior and accessibility implementation for a section in a grid list. | ||
* See `useGridList` for more details about grid list. | ||
* @param props - Props for the section. | ||
*/ | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
export function useGridListSection<T>(props: AriaGridListSectionProps, state: ListState<T>, ref: RefObject<HTMLElement | null>): GridListSectionAria { | ||
let {'aria-label': ariaLabel} = props; | ||
let headingId = useSlotId(); | ||
let labelProps = useLabels({ | ||
'aria-label': ariaLabel, | ||
'aria-labelledby': headingId | ||
}); | ||
|
||
return { | ||
rowProps: { | ||
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. so ideally it'd be easy to determine if we calculated it inside GridListHeader but that wouldn't match any of our existing API and would sort of defeat the purpose of having this hook supply this information... 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. just thinking out loud (and definitely don't implement this yet), but is it possible to:
I think the ideal way to calculate this will come from the collection nodes containing more information than they do now, include ways to get the "indexOfSameType" kind of information, something that maybe easier with some of the BaseCollection work in some of my other PRs 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. yeah i think we could just do a sum. i can look into it a bit more |
||
role: 'row' | ||
}, | ||
rowHeaderProps: { | ||
id: headingId, | ||
role: 'rowheader' | ||
}, | ||
rowGroupProps: { | ||
role: 'rowgroup', | ||
...labelProps | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,17 +9,18 @@ | |
* OF ANY KIND, either express or implied. See the License for the specific language | ||
* governing permissions and limitations under the License. | ||
*/ | ||
import {AriaGridListProps, DraggableItemResult, DragPreviewRenderer, DropIndicatorAria, DroppableCollectionResult, FocusScope, ListKeyboardDelegate, mergeProps, useCollator, useFocusRing, useGridList, useGridListItem, useGridListSelectionCheckbox, useHover, useLocale, useVisuallyHidden} from 'react-aria'; | ||
import {AriaGridListProps, DraggableItemResult, DragPreviewRenderer, DropIndicatorAria, DroppableCollectionResult, FocusScope, ListKeyboardDelegate, mergeProps, useCollator, useFocusRing, useGridList, useGridListItem, useGridListSection, useGridListSelectionCheckbox, useHover, useLocale, useVisuallyHidden} from 'react-aria'; | ||
import {ButtonContext} from './Button'; | ||
import {CheckboxContext} from './RSPContexts'; | ||
import {Collection, CollectionBuilder, createLeafComponent} from '@react-aria/collections'; | ||
import {CollectionProps, CollectionRendererContext, DefaultCollectionRenderer, ItemRenderProps} from './Collection'; | ||
import {Collection, CollectionBuilder, createBranchComponent, createLeafComponent} from '@react-aria/collections'; | ||
import {CollectionProps, CollectionRendererContext, DefaultCollectionRenderer, ItemRenderProps, SectionProps} from './Collection'; | ||
import {ContextValue, DEFAULT_SLOT, Provider, RenderProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps} from './utils'; | ||
import {DragAndDropContext, DropIndicatorContext, DropIndicatorProps, useDndPersistedKeys, useRenderDropIndicator} from './DragAndDrop'; | ||
import {DragAndDropHooks} from './useDragAndDrop'; | ||
import {DraggableCollectionState, DroppableCollectionState, Collection as ICollection, ListState, Node, SelectionBehavior, useListState} from 'react-stately'; | ||
import {filterDOMProps, inertValue, LoadMoreSentinelProps, useLoadMoreSentinel, useObjectRef} from '@react-aria/utils'; | ||
import {forwardRefType, GlobalDOMAttributes, HoverEvents, Key, LinkDOMProps, PressEvents, RefObject} from '@react-types/shared'; | ||
import {HeaderContext} from './Header'; | ||
import {ListStateContext} from './ListBox'; | ||
import React, {createContext, ForwardedRef, forwardRef, HTMLAttributes, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; | ||
import {TextContext} from './Text'; | ||
|
@@ -561,3 +562,58 @@ export const GridListLoadMoreItem = createLeafComponent('loader', function GridL | |
</> | ||
); | ||
}); | ||
|
||
export interface GridListSectionProps<T> extends SectionProps<T> {} | ||
|
||
/** | ||
* A GridListSection represents a section within a GridList. | ||
*/ | ||
export const GridListSection = /*#__PURE__*/ createBranchComponent('section', <T extends object>(props: GridListSectionProps<T>, ref: ForwardedRef<HTMLElement>, item: Node<T>) => { | ||
let state = useContext(ListStateContext)!; | ||
let {CollectionBranch} = useContext(CollectionRendererContext); | ||
let headingRef = useRef(null); | ||
ref = useObjectRef<HTMLElement>(ref); | ||
let {rowHeaderProps, rowProps, rowGroupProps} = useGridListSection({ | ||
'aria-label': props['aria-label'] ?? undefined | ||
}, state, ref); | ||
let renderProps = useRenderProps({ | ||
defaultClassName: 'react-aria-GridListSection', | ||
className: props.className, | ||
style: props.style, | ||
values: {} | ||
}); | ||
|
||
let DOMProps = filterDOMProps(props as any, {global: true}); | ||
delete DOMProps.id; | ||
|
||
return ( | ||
<section | ||
{...mergeProps(DOMProps, renderProps, rowGroupProps)} | ||
ref={ref}> | ||
<Provider | ||
values={[ | ||
[HeaderContext, {...rowProps, ref: headingRef}], | ||
[GridListHeaderContext, {...rowHeaderProps}] | ||
]}> | ||
<CollectionBranch | ||
collection={state.collection} | ||
parent={item} /> | ||
</Provider> | ||
</section> | ||
); | ||
}); | ||
|
||
const GridListHeaderContext = createContext<HTMLAttributes<HTMLElement> | null>(null); | ||
|
||
export const GridListHeader = /*#__PURE__*/ createLeafComponent('header', function Header(props: HTMLAttributes<HTMLElement>, ref: ForwardedRef<HTMLElement>) { | ||
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. with ListBox, you can just use a normal from WAI-ARIA: Each cell is either a DOM descendant of or owned by a row element and has one of the following roles:
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. I think this is ok, but the
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[props, ref] = useContextProps(props, ref, HeaderContext); | ||
let rowHeaderProps = useContext(GridListHeaderContext); | ||
|
||
return ( | ||
<header {...props} ref={ref}> | ||
<div {...rowHeaderProps} style={{display: 'contents'}}> | ||
{props.children} | ||
</div> | ||
</header> | ||
); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i did end up changing how we calculate rowIndex for rows inside Sections since Safari VO would read out the rows incorrectly otherwise. for example, say you were on the first item in Section 3, instead of saying something like "row 8 of 12" it would say "row 1 of 12"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if it would reasonable to use
aria-posinset
like tree does, did you try that before?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the way we calculate posinset for tree is also
node.index + 1
so i'm not sure if that would be any different. either way, i think the calculation for the index/posinset will need to be adjusted unless you mean something else?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but
aria-posinset
uses the relative position scoped to the parent section which I believe is what thenode.index
is currently giving you essentially. Not entirely sure if it appropriate here since these are sections, but you can see it being calculated per level here: https://www.w3.org/WAI/ARIA/apg/patterns/treegrid/examples/treegrid-1/There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if we just had
aria-posinset
(so no aria-rowindex) then it seems to announce fine, but if you had a combo of aria-posinset and aria-rowindex where both are calculated node.index + 1, then the announcements are wrong. seems like we should probably have aria-rowindex based on https://w3c.github.io/aria/#aria-rowindex so i think regardless, we'll need to update how rowindex is calculated