feat(react-tags): decouple useTagGroupBase_unstable from Tabster + export contexts #36228
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "minor", | ||
| "comment": "feat: export contexts for headless", | ||
| "packageName": "@fluentui/react-tags", | ||
| "email": "vgenaev@gmail.com", | ||
| "dependentChangeType": "patch" | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,12 @@ import { useArrowNavigationGroup, useFocusFinders } from '@fluentui/react-tabste | |
| import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; | ||
| import { interactionTagSecondaryClassNames } from '../InteractionTagSecondary/useInteractionTagSecondaryStyles.styles'; | ||
| import type { TagValue } from '../../utils/types'; | ||
| import type { TabsterDOMAttribute } from '@fluentui/react-tabster'; | ||
|
|
||
| type UseTagGroupBaseOptions = { | ||
|
Contributor
There was a problem hiding this comment. why cant we extend BaseProps type with this and have it all passed through props without need to change hook api pattern (which we probably should not do) ? |
||
| arrowNavigationProps?: TabsterDOMAttribute; | ||
| onAfterTagDismiss?: (container: HTMLElement | null) => void; | ||
| }; | ||
|
|
||
| /** | ||
| * Create the base state required to render TagGroup, without design-only props. | ||
|
|
@@ -24,6 +30,7 @@ import type { TagValue } from '../../utils/types'; | |
| export const useTagGroupBase_unstable = ( | ||
| props: TagGroupBaseProps, | ||
| ref: React.Ref<HTMLDivElement>, | ||
| options?: UseTagGroupBaseOptions, | ||
|
Member
There was a problem hiding this comment. Adding third argument is an architecture change. I would like to avoid it
Contributor
There was a problem hiding this comment. good callout , do we have this api change in any of our existing base hooks ? if not this should be avoided
Contributor
There was a problem hiding this comment.
Member
There was a problem hiding this comment. true, but it leaked from the similar PR #36087 and should be undone
Contributor
There was a problem hiding this comment. we will ship lint rule for this to prevent these regressions |
||
| ): TagGroupBaseState => { | ||
| const { | ||
| onDismiss, | ||
|
|
@@ -37,8 +44,6 @@ export const useTagGroupBase_unstable = ( | |
| } = props; | ||
|
|
||
| const innerRef = React.useRef<HTMLElement>(undefined); | ||
|
|
||
| const [items, setItems] = useControllableState<Array<TagValue>>({ | ||
| defaultState: defaultSelectedValues, | ||
|
|
@@ -48,26 +53,7 @@ export const useTagGroupBase_unstable = ( | |
|
|
||
| const handleTagDismiss: TagGroupBaseState['handleTagDismiss'] = useEventCallback((e, data) => { | ||
| onDismiss?.(e, data); | ||
|
Member
There was a problem hiding this comment.
|
||
| options?.onAfterTagDismiss?.(innerRef.current ?? null); | ||
| }); | ||
|
|
||
| const handleTagSelect: TagGroupBaseState['handleTagSelect'] = useEventCallback( | ||
|
|
@@ -80,12 +66,6 @@ export const useTagGroupBase_unstable = ( | |
| }), | ||
| ); | ||
|
|
||
| return { | ||
| handleTagDismiss, | ||
| handleTagSelect: onTagSelect ? handleTagSelect : undefined, | ||
|
|
@@ -105,7 +85,7 @@ export const useTagGroupBase_unstable = ( | |
| ref: useMergedRefs(ref, innerRef) as React.Ref<HTMLDivElement>, | ||
| role, | ||
| 'aria-disabled': disabled, | ||
| ...options?.arrowNavigationProps, | ||
|
Member
There was a problem hiding this comment. This could be just spreaded to |
||
| ...rest, | ||
| }), | ||
| { elementType: 'div' }, | ||
|
|
@@ -124,8 +104,39 @@ export const useTagGroupBase_unstable = ( | |
| */ | ||
| export const useTagGroup_unstable = (props: TagGroupProps, ref: React.Ref<HTMLDivElement>): TagGroupState => { | ||
| const { size = 'medium', appearance = 'filled' } = props; | ||
|
|
||
| const { targetDocument } = useFluent(); | ||
| const { findNextFocusable, findPrevFocusable } = useFocusFinders(); | ||
|
|
||
| const arrowNavigationProps = useArrowNavigationGroup({ | ||
| circular: true, | ||
| axis: 'both', | ||
| memorizeCurrent: true, | ||
| }); | ||
|
|
||
| const onAfterTagDismiss = useEventCallback((container: HTMLElement | null) => { | ||
| const activeElement = targetDocument?.activeElement; | ||
| if (container?.contains(activeElement as HTMLElement)) { | ||
| // focus on next tag only if the active element is within the current tag group | ||
| const next = findNextFocusable(activeElement as HTMLElement, { container }); | ||
| if (next) { | ||
| next.focus(); | ||
| return; | ||
| } | ||
|
|
||
| // if there is no next focusable, focus on the previous focusable | ||
| if (activeElement?.className.includes(interactionTagSecondaryClassNames.root)) { | ||
| const prev = findPrevFocusable(activeElement.parentElement as HTMLElement, { container }); | ||
| prev?.focus(); | ||
| } else { | ||
| const prev = findPrevFocusable(activeElement as HTMLElement, { container }); | ||
| prev?.focus(); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return { | ||
| ...useTagGroupBase_unstable(props, ref, { arrowNavigationProps, onAfterTagDismiss }), | ||
| size, | ||
| appearance, | ||
| }; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Avatar Converged 1 screenshots
vr-tests-react-components/Charts-DonutChart 1 screenshots
vr-tests-react-components/Menu Converged - submenuIndicator slotted content 2 screenshots
vr-tests-react-components/Positioning 2 screenshots
vr-tests-react-components/ProgressBar converged 2 screenshots
vr-tests-react-components/TagPicker 1 screenshots
There were 1 duplicate changes discarded. Check the build logs for more information.