-
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 5 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,58 @@ | ||
/* | ||
* 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} from '@react-types/shared'; | ||
import {ReactNode} from 'react'; | ||
import {useId} from '@react-aria/utils'; | ||
|
||
export interface AriaGridListSectionProps { | ||
/** The heading for the section. */ | ||
heading?: ReactNode, | ||
/** 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. */ | ||
headingProps: 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. | ||
*/ | ||
export function useGridListSection(props: AriaGridListSectionProps): GridListSectionAria { | ||
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. pass state and ref, we always seem to regret not passing them and it's breaking to add them later 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. we could pass it but wouldn't they be unused? 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. still need to declare them in the signature so that others using the hook know they should supply them. that's the only way it can be non-breaking |
||
let {heading, 'aria-label': ariaLabel} = props; | ||
let headingId = useId(); | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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' | ||
}, | ||
headingProps: heading ? { | ||
id: headingId, | ||
role: 'rowheader' | ||
} : {}, | ||
rowGroupProps: { | ||
role: 'rowgroup', | ||
'aria-label': ariaLabel, | ||
'aria-labelledby': heading ? headingId : undefined | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
}; | ||
} |
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 {ContextValue, DEFAULT_SLOT, Provider, RenderProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps} from './utils'; | ||
import {Collection, CollectionBuilder, createBranchComponent, createLeafComponent} from '@react-aria/collections'; | ||
import {CollectionProps, CollectionRendererContext, DefaultCollectionRenderer, ItemRenderProps, SectionContext, SectionProps} from './Collection'; | ||
import {ContextValue, DEFAULT_SLOT, Provider, RenderProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps, useSlot} 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'; | ||
|
@@ -245,7 +246,8 @@ function GridListInner<T extends object>({props, collection, gridListRef: ref}: | |
values={[ | ||
[ListStateContext, state], | ||
[DragAndDropContext, {dragAndDropHooks, dragState, dropState}], | ||
[DropIndicatorContext, {render: GridListDropIndicatorWrapper}] | ||
[DropIndicatorContext, {render: GridListDropIndicatorWrapper}], | ||
[SectionContext, {name: 'GridListSection', render: GridListSectionInner}] | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
]}> | ||
{isListDroppable && <RootDropIndicator />} | ||
<CollectionRoot | ||
|
@@ -561,3 +563,62 @@ export const GridListLoadMoreItem = createLeafComponent('loader', function GridL | |
</> | ||
); | ||
}); | ||
|
||
export interface GridListSectionProps<T> extends SectionProps<T> {} | ||
|
||
function GridListSectionInner<T extends object>(props: GridListSectionProps<T>, ref: ForwardedRef<HTMLElement>, section: Node<T>, className = 'react-aria-GridListSection') { | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let state = useContext(ListStateContext)!; | ||
let {dragAndDropHooks, dropState} = useContext(DragAndDropContext)!; | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let {CollectionBranch} = useContext(CollectionRendererContext); | ||
let [headingRef, heading] = useSlot(); | ||
let {headingProps, rowProps, rowGroupProps} = useGridListSection({ | ||
heading, | ||
'aria-label': props['aria-label'] ?? undefined | ||
}); | ||
let renderProps = useRenderProps({ | ||
defaultClassName: className, | ||
className: props.className, | ||
style: props.style, | ||
values: {} | ||
}); | ||
|
||
return ( | ||
<section | ||
{...filterDOMProps(props as any)} | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{...rowGroupProps} | ||
{...renderProps} | ||
yihuiliao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ref={ref}> | ||
<Provider | ||
values={[ | ||
[HeaderContext, {...headingProps, ref: headingRef}], | ||
[GridListHeaderContext, {...rowProps}] | ||
]}> | ||
<CollectionBranch | ||
collection={state.collection} | ||
parent={section} | ||
renderDropIndicator={useRenderDropIndicator(dragAndDropHooks, dropState)} /> | ||
</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 rowProps = useContext(GridListHeaderContext); | ||
|
||
return ( | ||
<div {...rowProps} > | ||
<header className="react-aria-Header" {...props} ref={ref}> | ||
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. Having two elements will make this harder to style. Maybe we can use <header {...rowProps} ref={ref}>
<div {...rowHeaderProps} style={{display: 'contents'}}>
{children}
</div>
</header> This would match the structure of 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. linking related discussion #8667 (comment) I think I like the display contents better 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. +1, I did the same for the load more elements too (i.e. TableLoadMoreItem) 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. @snowystinger Regarding #8667 (comment) I think we could provide a custom CollectionNode to wrap headers children with the row div. Otherwise we could expose an internal context in 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.
Definitely an interesting idea. Though I imagine that would have the same issue with styling, so we'd probably still use 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, styling issue persists. This just enables re-use of 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. We've decided as a team to go ahead and use 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. fine with that 👍 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 so we have the history, it was noted that we'd need a custom renderer as well in order to reuse the |
||
{props.children} | ||
</header> | ||
</div> | ||
); | ||
}); | ||
|
||
|
||
/** | ||
* A GridListSection represents a section within a GridList. | ||
*/ | ||
export const GridListSection = /*#__PURE__*/ createBranchComponent('section', GridListSectionInner); |
Uh oh!
There was an error while loading. Please reload this page.