Skip to content

Commit bb99f85

Browse files
adamhaegeradamhaeger
committed
cleanup
1 parent 05a0b5b commit bb99f85

File tree

4 files changed

+64
-37
lines changed

4 files changed

+64
-37
lines changed

‎src/app-components/TimePicker/TimePicker.module.css‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,6 @@
141141
z-index: 1;
142142
}
143143

144-
.dropdownOptionFocused.dropdownOptionSelected {
145-
/* When option is both focused and selected, prioritize selection styling but add focus outline */
146-
/*outline: 2px solid var(--ds-color-neutral-text-on-inverted);*/
147-
/*outline-offset: -2px;*/
148-
}
149-
150144
.dropdownOptionDisabled {
151145
opacity: 0.5;
152146
cursor: not-allowed;
@@ -180,7 +174,7 @@
180174
@media (max-width: 348px) {
181175
.calendarInputWrapper {
182176
flex-wrap: wrap;
183-
gap: var(--ds-size-0-5);
177+
gap: var(--ds-size-1);
184178
}
185179

186180
.segmentContainer {

‎src/app-components/TimePicker/TimePicker.tsx‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
5555
const timeValue = parseTimeString(value, format);
5656

5757
const [showDropdown, setShowDropdown] = useState(false);
58-
const [_focusedSegment, setFocusedSegment] = useState<number | null>(null);
5958

6059
// Dropdown keyboard navigation state
6160
const [dropdownFocus, setDropdownFocus] = useState<DropdownFocusState>({
@@ -89,7 +88,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
8988
maxTime,
9089
};
9190

92-
// Segment labels and placeholders
9391
const segmentLabels = {
9492
hours: labels.hours || 'Hours',
9593
minutes: labels.minutes || 'Minutes',
@@ -105,7 +103,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
105103
};
106104

107105
const scrollToSelectedOptions = () => {
108-
// Use requestAnimationFrame to ensure DOM is ready
109106
requestAnimationFrame(() => {
110107
const scrollToSelected = (container: HTMLDivElement | null) => {
111108
if (!container) {
@@ -156,7 +153,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
156153
}
157154

158155
segmentRefs.current[nextIndex]?.focus();
159-
setFocusedSegment(nextIndex);
160156
};
161157

162158
const closeDropdown = () => {
@@ -438,8 +434,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
438434
format={format}
439435
onValueChange={(newValue) => handleSegmentChange(segmentType, newValue)}
440436
onNavigate={(direction) => handleSegmentNavigate(direction, index)}
441-
onFocus={() => setFocusedSegment(index)}
442-
onBlur={() => setFocusedSegment(null)}
443437
placeholder={segmentPlaceholders[segmentType]}
444438
disabled={disabled}
445439
readOnly={readOnly}
@@ -462,7 +456,9 @@ export const TimePicker: React.FC<TimePickerProps> = ({
462456
aria-controls={`${id}-dropdown`}
463457
disabled={disabled || readOnly}
464458
data-size='sm'
465-
onClick={() => setShowDropdown(!showDropdown)}
459+
onClick={() => {
460+
setShowDropdown(!showDropdown);
461+
}}
466462
>
467463
<ClockIcon />
468464
</Popover.Trigger>
@@ -497,7 +493,6 @@ export const TimePicker: React.FC<TimePickerProps> = ({
497493
}}
498494
onClose={() => {
499495
closeDropdown();
500-
triggerButtonRef.current?.focus({ preventScroll: true });
501496
}}
502497
onKeyDown={handleDropdownKeyDown}
503498
tabIndex={0}
@@ -545,7 +540,17 @@ export const TimePicker: React.FC<TimePickerProps> = ({
545540
} ${isFocused ? styles.dropdownOptionFocused : ''} ${
546541
isDisabled ? styles.dropdownOptionDisabled : ''
547542
}`}
548-
onClick={() => !isDisabled && handleDropdownHoursChange(option.value.toString())}
543+
onClick={() => {
544+
if (!isDisabled) {
545+
handleDropdownHoursChange(option.value.toString());
546+
// Update focus to the clicked option
547+
setDropdownFocus({
548+
column: 0,
549+
option: optionIndex,
550+
isActive: true,
551+
});
552+
}
553+
}}
549554
disabled={isDisabled}
550555
aria-label={option.label}
551556
>
@@ -590,7 +595,17 @@ export const TimePicker: React.FC<TimePickerProps> = ({
590595
} ${isFocused ? styles.dropdownOptionFocused : ''} ${
591596
isDisabled ? styles.dropdownOptionDisabled : ''
592597
}`}
593-
onClick={() => !isDisabled && handleDropdownMinutesChange(option.value.toString())}
598+
onClick={() => {
599+
if (!isDisabled) {
600+
handleDropdownMinutesChange(option.value.toString());
601+
// Update focus to the clicked option
602+
setDropdownFocus({
603+
column: 1,
604+
option: optionIndex,
605+
isActive: true,
606+
});
607+
}
608+
}}
594609
disabled={isDisabled}
595610
aria-label={option.label}
596611
>
@@ -636,7 +651,17 @@ export const TimePicker: React.FC<TimePickerProps> = ({
636651
} ${isFocused ? styles.dropdownOptionFocused : ''} ${
637652
isDisabled ? styles.dropdownOptionDisabled : ''
638653
}`}
639-
onClick={() => !isDisabled && handleDropdownSecondsChange(option.value.toString())}
654+
onClick={() => {
655+
if (!isDisabled) {
656+
handleDropdownSecondsChange(option.value.toString());
657+
// Update focus to the clicked option
658+
setDropdownFocus({
659+
column: 2,
660+
option: optionIndex,
661+
isActive: true,
662+
});
663+
}
664+
}}
640665
disabled={isDisabled}
641666
aria-label={option.label}
642667
>
@@ -679,7 +704,14 @@ export const TimePicker: React.FC<TimePickerProps> = ({
679704
className={`${styles.dropdownOption} ${
680705
isSelected ? styles.dropdownOptionSelected : ''
681706
} ${isFocused ? styles.dropdownOptionFocused : ''}`}
682-
onClick={() => handleDropdownPeriodChange(period as 'AM' | 'PM')}
707+
onClick={() => {
708+
handleDropdownPeriodChange(period as 'AM' | 'PM');
709+
setDropdownFocus({
710+
column: columnIndex,
711+
option: optionIndex,
712+
isActive: true,
713+
});
714+
}}
683715
aria-label={period}
684716
>
685717
{period}

‎src/app-components/TimePicker/TimeSegment/TimeSegment.test.tsx‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,20 +256,19 @@ describe('TimeSegment Component', () => {
256256
});
257257

258258
describe('Focus Behavior', () => {
259-
it('should select all text on focus', async () => {
260-
const onFocus = jest.fn();
259+
it('should handle focus events', async () => {
261260
render(
262261
<TimeSegment
263262
{...defaultProps}
264263
value={12}
265-
onFocus={onFocus}
266264
/>,
267265
);
268266
const input = screen.getByRole('textbox') as HTMLInputElement;
269267

270268
await userEvent.click(input);
271269

272-
expect(onFocus).toHaveBeenCalled();
270+
// Just verify the input can receive focus
271+
expect(document.activeElement).toBe(input);
273272
// Note: Testing text selection is limited in jsdom
274273
});
275274

‎src/app-components/TimePicker/TimeSegment/TimeSegment.tsx‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ export interface TimeSegmentProps {
1616
format: TimeFormat;
1717
onValueChange: (value: number | string) => void;
1818
onNavigate: (direction: 'left' | 'right') => void;
19-
onFocus?: () => void;
2019
onBlur?: () => void;
2120
placeholder?: string;
2221
disabled?: boolean;
@@ -35,15 +34,14 @@ export const TimeSegment = React.forwardRef<HTMLInputElement, TimeSegmentProps>(
3534
format,
3635
onValueChange,
3736
onNavigate,
38-
onFocus,
3937
onBlur,
4038
placeholder,
4139
disabled,
4240
readOnly,
4341
required,
4442
'aria-label': ariaLabel,
4543
className,
46-
autoFocus,
44+
// autoFocus,
4745
},
4846
ref,
4947
) => {
@@ -65,13 +63,15 @@ export const TimeSegment = React.forwardRef<HTMLInputElement, TimeSegmentProps>(
6563
});
6664

6765
const syncExternalChangesWhenNotTyping = () => {
66+
console.log('syncExternalChangesWhenNotTyping called, isTyping:', typingBuffer.isTyping);
6867
if (!typingBuffer.isTyping) {
68+
console.log('syncing external value and resetting to idle');
6969
syncWithExternalValue();
7070
typingBuffer.resetToIdleState();
7171
}
7272
};
7373

74-
React.useEffect(syncExternalChangesWhenNotTyping, [value, type, format, typingBuffer, syncWithExternalValue]);
74+
React.useEffect(syncExternalChangesWhenNotTyping, [value, type, format, syncWithExternalValue, typingBuffer]);
7575

7676
const handleCharacterTyping = (event: React.KeyboardEvent<HTMLInputElement>) => {
7777
const character = event.key;
@@ -108,23 +108,24 @@ export const TimeSegment = React.forwardRef<HTMLInputElement, TimeSegmentProps>(
108108
}
109109
};
110110

111-
const handleFocusEvent = (event: React.FocusEvent<HTMLInputElement>) => {
112-
const wasFreshFocus = event.currentTarget !== document.activeElement;
113-
114-
if (wasFreshFocus) {
115-
typingBuffer.resetToIdleState();
116-
event.target.select();
117-
}
118-
119-
onFocus?.();
111+
const handleFocusEvent = (_: React.FocusEvent<HTMLInputElement>) => {
112+
console.log('focus');
113+
// Don't reset typing buffer on focus as it causes issues
114+
// Let the user focus and type naturally
120115
};
121116

122117
const handleBlurEvent = () => {
118+
console.log('blur');
123119
typingBuffer.commitImmediatelyAndEndTyping();
124120
inputHandlers.fillEmptyMinutesOrSecondsWithZero();
125121
onBlur?.();
126122
};
127123

124+
const handleClick = (_: React.MouseEvent<HTMLInputElement>) => {
125+
// Ensure the input gets focus when clicked
126+
// event.currentTarget.focus();
127+
};
128+
128129
return (
129130
<Textfield
130131
ref={ref}
@@ -135,13 +136,14 @@ export const TimeSegment = React.forwardRef<HTMLInputElement, TimeSegmentProps>(
135136
onKeyDown={handleSpecialKeys}
136137
onFocus={handleFocusEvent}
137138
onBlur={handleBlurEvent}
139+
onClick={handleClick}
138140
placeholder={placeholder}
139141
disabled={disabled}
140142
readOnly={readOnly}
141143
required={required}
142144
aria-label={ariaLabel}
143145
className={className}
144-
autoFocus={autoFocus}
146+
autoFocus={false}
145147
data-size='sm'
146148
style={{
147149
width: type === 'period' ? '4rem' : '3rem',

0 commit comments

Comments
 (0)