Add accessibility props to scroll container#432
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #432 +/- ##
=======================================
Coverage 95.87% 95.87%
=======================================
Files 4 4
Lines 194 194
Branches 65 65
=======================================
Hits 186 186
Misses 4 4
Partials 4 4 🚀 New features to boost your workflow:
|
|
Hey @sayedrisat, thanks for picking this up, the implementation is clean and the test covers the right element. One thing I'd love your thoughts on before we merge: the current approach extends What do you think about scoping it down to just what's needed for accessibility? export interface Props extends React.AriaAttributes {
// existing props ...
role?: React.AriaRole;
tabIndex?: number;
id?: string;
}
The destructure would change to collecting the remainder as Happy to be wrong here if there's a use case for the wider surface that I'm missing. What's your take? |
|
Thanks for the thoughtful review. I agree with keeping this scoped to the accessibility use case instead of exposing the full div attribute surface. I pushed an update that replaces the broad HTMLAttributes extension with React.AriaAttributes plus explicit role, tabIndex, and id props. The component now forwards the collected aria props along with those named props to the scroll container, and I updated the regression test and README docs to match. Verified locally with:
|
|
One more thing on the README before we merge, two small changes would make this a lot more useful for consumers: 1. Split the catch-all row into individual props The current "Accessibility attributes" row won't be found by someone scanning the table for 2. Add a short Accessibility section with usage examples The props table tells you what's available but not how to wire it up correctly. Since this whole PR is about screen reader support, a minimal example would save consumers a trip to the ARIA spec: ## Accessibility
Pass `role` and a label so screen readers can announce the container correctly:
```tsx
<InfiniteScroll
role="list"
aria-label="Search results"
dataLength={items.length}
next={fetchMore}
hasMore={hasMore}
loader={<p>Loading...</p>}
>
{items.map(item => (
<div role="listitem" key={item.id}>{item.name}</div>
))}
</InfiniteScroll>Or reference an existing heading via <h2 id="results-heading">Search results</h2>
<InfiniteScroll
role="list"
aria-labelledby="results-heading"
...
>Neither of these touches any code, just the README. Good to merge once these are in. |
Closes #411. Summary: Extends InfiniteScroll props to accept standard div attributes and forwards them to the inner scroll container, enabling role, aria-label, and tabIndex for accessibility. Added a regression test and README prop documentation. Verification: yarn test --runInBand; yarn test src/tests/index.test.tsx --runInBand; yarn ts-check; yarn build; prettier check on touched files; direct eslint command completed with warnings only (existing warnings).