Skip to content
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

Wrong type for DayProps[rootProps] #2223

Closed
ArthurGoupil opened this issue Jun 24, 2024 · 6 comments
Closed

Wrong type for DayProps[rootProps] #2223

ArthurGoupil opened this issue Jun 24, 2024 · 6 comments
Milestone

Comments

@ArthurGoupil
Copy link
Contributor

ArthurGoupil commented Jun 24, 2024

Description

Hello!

In DayProps, the provided rootProps are supposed to be applied on a div.

export declare function Day(props: {
    day: CalendarDay;
    modifiers: DayModifiers;
    children?: ReactNode;
    rootProps: Pick<JSX.IntrinsicElements["div"], "className" | "style" | "tabIndex" | "aria-colindex" | "aria-disabled" | "aria-hidden" | "aria-label" | "aria-selected" | "onClick" | "onBlur" | "onFocus" | "onKeyDown" | "onKeyPress" | "onKeyUp" | "onMouseEnter" | "onMouseLeave" | "onPointerEnter" | "onPointerLeave" | "onTouchCancel" | "onTouchEnd" | "onTouchMove" | "onTouchStart" | "ref" | "role">;
}): React.JSX.Element;
export type DayProps = Parameters<typeof Day>[0];

If I'm not wrong this is not great for accessibility:

@gpbl
Copy link
Owner

gpbl commented Jun 24, 2024

Hi @ArthurGoupil thanks for your feedback - in this case I believe the linter warning is a false positive, as the interactive element is marked as a grid cell and reacts to user interactions - such as click or focus.

@gpbl gpbl removed their assignment Jun 24, 2024
@ArthurGoupil
Copy link
Contributor Author

Not sure to understand, even with the gridcell role, the div is not supposed to be interactive right?

Actually, I wanted to implement the following behaviour: for a vertical 6 months calendar, display directly the selected date even if it's 3 months later (so automatic scroll if needed).
The only way I found to handle this is with an autofocus. The autofocus doesn't work with the gridcell div, while it does with a button.

@Benrajalu
Copy link

Hello @gpbl, I don't understand the correlation between the role="gridcell" and interactivity 🤔 If anything the current output is <div role="gridcell" onClick="..." tabIndex="...."> but gridcell is not parsed as interactive by screen readers 🤔

Running Voiceover on this https://codesandbox.io/p/sandbox/rendering-test-react-day-picker-v9-3kwpm8?file=/src/index.tsx I never get prompted to click, only the day's number is being read.

@gpbl
Copy link
Owner

gpbl commented Jun 25, 2024

@Benrajalu thanks for the feedback, yeah I agree we should consider again the button there.

What is your suggestion here? What is the correct ARIA tree?

I'll try with a grid cell wrapping a Button element, hopefully will make VoiceOver happier.

<div role="gridcell" aria-selected={true}>
  <button tabIndex={1}> 
    {date}
  </button>
</div>

@Benrajalu
Copy link

That does seem better yes, gridCell for the layout, button for the interaction!

@gpbl gpbl added this to the v9.0.0 milestone Jul 7, 2024
@gpbl
Copy link
Owner

gpbl commented Jul 7, 2024

Thanks a lot for your suggestions. In 9.0.0-rc.3 we are back to tables with buttons and Now screen readers are more clear when describing the component.

Screenshot 2024-07-07 at 8 40 53 AM

I would appreciate someone testing this release for accessibility. Thanks.

@gpbl gpbl closed this as completed Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants