Skip to content

Commit 0f2ba5b

Browse files
Improve dropdown keyboard accessibility implementation
- Fix memory leak in month dropdown by clearing ref object on re-render - Add bounds checking for year navigation to prevent focusing non-existent elements - Use Record<number, Element> instead of array for more efficient ref management - Improve consistency between month and year dropdown implementations - Maintain 100% line coverage and improve branch coverage These improvements address code review feedback and enhance robustness. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 54c5fee commit 0f2ba5b

File tree

2 files changed

+12
-3
lines changed

2 files changed

+12
-3
lines changed

src/month_dropdown_options.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ interface MonthDropdownOptionsProps {
1010
}
1111

1212
export default class MonthDropdownOptions extends Component<MonthDropdownOptionsProps> {
13-
monthOptionButtonsRef: (HTMLDivElement | null)[] = [];
13+
monthOptionButtonsRef: Record<number, HTMLDivElement | null> = {};
1414

1515
isSelectedMonth = (i: number): boolean => this.props.month === i;
1616

@@ -37,11 +37,14 @@ export default class MonthDropdownOptions extends Component<MonthDropdownOptions
3737
};
3838

3939
renderOptions = (): React.ReactElement[] => {
40+
// Clear refs to prevent memory leaks on re-render
41+
this.monthOptionButtonsRef = {};
42+
4043
return this.props.monthNames.map<React.ReactElement>(
4144
(month: string, i: number): React.ReactElement => (
4245
<div
4346
ref={(el) => {
44-
this.monthOptionButtonsRef?.push(el);
47+
this.monthOptionButtonsRef[i] = el;
4548
if (this.isSelectedMonth(i)) {
4649
el?.focus();
4750
}

src/year_dropdown_options.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,19 @@ export default class YearDropdownOptions extends Component<
104104
case "ArrowDown": {
105105
e.preventDefault();
106106
const newYear = year + (e.key === "ArrowUp" ? 1 : -1);
107-
this.yearOptionButtonsRef[newYear]?.focus();
107+
// Add bounds checking to ensure the year exists in our refs
108+
if (this.yearOptionButtonsRef[newYear]) {
109+
this.yearOptionButtonsRef[newYear]?.focus();
110+
}
108111
break;
109112
}
110113
}
111114
};
112115

113116
renderOptions = (): React.ReactElement[] => {
117+
// Clear refs to prevent memory leaks on re-render
118+
this.yearOptionButtonsRef = {};
119+
114120
const selectedYear = this.props.year;
115121
const options = this.state.yearsList.map((year) => (
116122
<div

0 commit comments

Comments
 (0)