Skip to content

refactor: extract shared base for year/month/month-year dropdowns - #92

Open
balajis-qb wants to merge 1 commit into
mainfrom
refactor/extract-shared-dropdown-base
Open

balajis-qb wants to merge 1 commit into
mainfrom
refactor/extract-shared-dropdown-base

Conversation

@balajis-qb

Copy link
Copy Markdown
Collaborator

Summary

Structural cleanup — no linked issue. month_dropdown.tsx, year_dropdown.tsx, and month_year_dropdown.tsx were nearly identical and had already drifted apart in a way that broke accessibility for one of them.

Problem

  • month_dropdown.tsx, year_dropdown.tsx, and month_year_dropdown.tsx were ~90% duplicated: same dropdownVisible state shape, same read-view / select-mode / scroll-mode / toggle skeleton — only the rendered options differed.
  • That duplication had already let one copy silently drift and cause a real bug: month_year_dropdown.tsx's read view was a plain <div onClick> instead of a <button type="button"> like the other two, so it wasn't reachable via Tab and didn't respond to Enter/Space (tracked upstream as issue showMonthYearDropdown read view not reachable via Tab / keyboard Hacker0x01/react-datepicker#6311).
  • Fix: extract the shared pieces (a DropdownReadView button, the BEM container class builder, and the scroll-mode array assembly) into src/dropdown_common.tsx, and have all three components use it. Unifying onto one read-view implementation fixes the div-vs-button bug as a structural side effect of removing the duplication, not as a separate targeted patch.

Changes

  • Add src/dropdown_common.tsx: DropdownReadView (shared toggle button), dropdownContainerClassName, renderScrollModeChildren.
  • src/year_dropdown.tsx, src/month_dropdown.tsx, src/month_year_dropdown.tsx: replace hand-copied read-view/container-class/scroll-mode JSX with the shared helpers.
  • src/test/dropdown_common.test.tsx (new): direct test coverage for the extracted module.
  • src/test/month_year_dropdown_test.test.tsx: add a regression test asserting the read view renders as a real <button type="button">.

Screenshots

Not applicable — This is a structural refactor with no visual change. The same CSS classes and markup shape are reused; the only element-tag change (div → button) is covered by tests rather than being visually apparent.

Contribution checklist

  • I have followed the contributing guidelines.
  • I have added sufficient test coverage for my changes.
  • I have formatted my code with Prettier and checked for linting issues with ESLint for code readability.

month_dropdown.tsx, year_dropdown.tsx, and month_year_dropdown.tsx were
~90% duplicated: same dropdownVisible state shape, same read-view/select-
mode/scroll-mode/toggle skeleton, differing only in what options they
render. That divergence had already caused a real bug (Hacker0x01#6311):
month_year_dropdown's read view was a plain <div onClick>, unlike the
other two which used <button type="button">, so it was unreachable via
Tab and didn't respond to Enter/Space.

Add dropdown_common.tsx (DropdownReadView, dropdownContainerClassName,
renderScrollModeChildren) and use it in all three components. Unifying
the read view onto a real <button> fixes Hacker0x01#6311 as a structural side
effect of removing the duplication, not just a targeted patch.
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

Successfully merging this pull request may close these issues.

1 participant