Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances accessibility and keyboard navigation for media control components while refactoring button components for improved type safety. The changes focus on making sliders keyboard focusable, improving focus visibility, and cleaning up type definitions.
- Added keyboard focus support (
tabIndex={0}) to TimeSlider and VolumeSlider components - Enhanced focus styling with rounded corners and outline offsets for better visual accessibility
- Refactored button components (FullscreenButton, MuteButton, PlayButton) with cleaner type definitions and renamed prop hooks
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/react/src/components/TimeSlider.tsx | Added tabIndex and corrected ARIA attributes for keyboard accessibility |
| packages/react/react/src/components/VolumeSlider.tsx | Added tabIndex and corrected ARIA attributes for keyboard accessibility |
| packages/react/react/src/components/Popover.tsx | Integrated FloatingFocusManager for better focus management and added default portal ID |
| packages/react/react/src/skins/default/styles.ts | Added focus styles for slider roots and specific TimeSlider/VolumeSlider styling |
| packages/react/react/src/skins/toasted/styles.ts | Enhanced focus styling with rounded corners and outline offsets |
| packages/react/react/src/skins/default/types.ts | Added new style properties for TimeSliderRoot and VolumeSliderRoot |
| packages/react/react/src/skins/default/MediaSkinDefault.tsx | Applied new style classes to slider components |
| packages/react/react/src/components/FullscreenButton.tsx | Refactored types and added button type attribute |
| packages/react/react/src/components/MuteButton.tsx | Refactored types and added button type attribute |
| packages/react/react/src/components/PlayButton.tsx | Refactored types and added button type attribute |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| function PopoverPortal({ children, root, rootId }: PopoverPortalProps): JSX.Element { | ||
| function PopoverPortal({ children, root, rootId = '@default_portal_id' }: PopoverPortalProps): JSX.Element { |
There was a problem hiding this comment.
nit: should we stick to kebab case for IDs?
There was a problem hiding this comment.
good call, this is something @cjpillsbury added with the intention of removing in the future so not too worried about it.
This pull request refactors several media control button components (
FullscreenButton,MuteButton,PlayButton) in the React package for improved type safety and consistency, enhances accessibility and keyboard navigation for sliders, and updates styling for better focus visibility and UI clarity. The changes also improve the popover's accessibility by managing focus more robustly.Component Refactoring and Type Improvements:
FullscreenButton,MuteButton, andPlayButtonto use more precise types (ReturnType<typeof ...>) and renamed prop hooks toget...Propsfor clarity and consistency. Updated connected component creation to use the new prop hook names. [1] [2] [3] [4] [5] [6]Accessibility and Keyboard Navigation:
tabIndex={0}to bothTimeSliderandVolumeSliderroot elements, making them keyboard focusable. Updated ARIA attributes to reflect actual slider values (e.g.,aria-valuenow,aria-valuemax) for better screen reader support. [1] [2]type="button"to button elements to prevent unintended form submissions and clarify intent. [1] [2] [3]Popover and Focus Management:
FloatingFocusManagerinto thePopover.Popupcomponent to manage keyboard focus when the popover is open, improving accessibility for keyboard and assistive technology users. Set the initial focus to the popover's reference element. [1] [2]PopoverPortalto ensure consistent portal mounting.Styling and Focus Visibility:
TimeSliderRootandVolumeSliderRootto control focus outline offsets. [1] [2] [3] [4]TimeSlider.RootandVolumeSlider.Rootusages. [1] [2]These changes collectively improve code maintainability, accessibility, and user experience across the media control components.