feat: react-gamepad-navigation implementation #286
Conversation
This file contains 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
marcosmoura
previously approved these changes
Jan 23, 2025
|
requesting review from @tudorpopams |
dmytrokirpa
reviewed
Feb 20, 2025
There was a problem hiding this comment.
Nice work @hectorjjb! I added a few comments after my initial review, here's a quick summary:
- Avoid using TS
enums; it's better to use type union or a map with values if you prefer. - Keep type imports consistent by using
import type {}where needed. - Make sure side effects like adding/removing listeners happen in useEffect.
change/@fluentui-contrib-react-gamepad-navigation-3f106efd-9c1e-4b90-a4a2-817c0bb10c61.json
Outdated
Show resolved
Hide resolved
packages/react-gamepad-navigation/src/hooks/useGamepadNavigationGroup.ts
Outdated
Show resolved
Hide resolved
| "options": { | ||
| "packageRoot": "dist\\{projectRoot}" | ||
| } | ||
| "publish": { |
| unstable_hasDefault, | ||
| }); | ||
| const groupperAttr = useFocusableGroup({ tabBehavior, ignoreDefaultKeydown }); | ||
| const gamepadNavDOMAttributes = useMergedTabsterAttributes_unstable( |
There was a problem hiding this comment.
@ling1726 @mshoho This hook marked as "internal" https://github.com/microsoft/fluentui/blob/master/packages/react-components/react-tabster/src/hooks/useMergeTabsterAttributes.ts#L8, is okay to use in the extension/cosumer package?
packages/react-gamepad-navigation/src/hooks/useGamepadNavigationGroup.ts
Outdated
Show resolved
Hide resolved
| // eslint-disable-next-line @typescript-eslint/no-namespace | ||
| namespace React { | ||
| interface KeyboardEvent { | ||
| readonly [syntheticKey]?: boolean; |
There was a problem hiding this comment.
Not sure it's a good practice to change/augment external types
…onGroup.ts Co-authored-by: Dmytro Kirpa <[email protected]>
…onGroup.ts Co-authored-by: Dmytro Kirpa <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Enabling gamepad navigation by using Gamepad Web API and useArrowNavigationGroup under the hood.
Dpad and left stick events are translated to MoverMoveFocusEvent (Arrow navigation)
and A & B buttons are translated into Enter & Escape GroupperMoveFocusEvent respectively.