Skip to content

Commit

Permalink
DatePicker, TextField: 1. fix open behavior, 2. support to onClick (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
AlbertCarreras authored Jan 6, 2025
1 parent 78b2435 commit 233d0b4
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/gestalt-datepicker/src/DatePicker.jsdom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('DatePicker', () => {

// eslint-disable-next-line testing-library/no-unnecessary-act -- We have to wrap the focus event in `act` since it does change the component's internal state
await act(async () => {
await fireEvent.focus(screen.getByDisplayValue('12/13/2018'));
await fireEvent.click(screen.getByDisplayValue('12/13/2018'));
});

// Test correct render of DatePicker popover
Expand Down
4 changes: 2 additions & 2 deletions packages/gestalt-datepicker/src/DatePicker/DateInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ const DateInputWithForwardRef = forwardRef<HTMLInputElement, Props>(function Dat
name={name}
onBlur={(data) => onBlur?.(data.event)}
onChange={(data) => onChange?.(data.event)}
onClick={onClick}
onFocus={(data) => {
onPassthroughFocus?.();
onFocus?.(data.event);
onClick?.();
}}
onKeyDown={(data) => onKeyDown?.(data.event)}
placeholder={placeholder}
Expand Down Expand Up @@ -114,10 +114,10 @@ const DateInputWithForwardRef = forwardRef<HTMLInputElement, Props>(function Dat
name={name}
onBlur={(data) => onBlur?.(data.event)}
onChange={(data) => onChange?.(data.event)}
onClick={onClick}
onFocus={(data) => {
onPassthroughFocus?.();
onFocus?.(data.event);
onClick?.();
}}
onKeyDown={(data) => onKeyDown?.(data.event)}
placeholder={placeholder}
Expand Down
7 changes: 7 additions & 0 deletions packages/gestalt/src/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ type Props = {
* Callback triggered when the value of the input changes.
*/
onChange: (arg1: { event: React.ChangeEvent<HTMLInputElement>; value: string }) => void;
/**
* Callback triggered when the user user clicks the input.
*/
onClick?: (arg1: { event: React.MouseEvent<HTMLInputElement>; value: string }) => void;
/**
* Callback triggered when the user focuses the input.
*/
Expand Down Expand Up @@ -138,6 +142,7 @@ const TextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(function Tex
name,
onBlur,
onChange,
onClick,
onFocus,
onKeyDown,
placeholder,
Expand Down Expand Up @@ -211,6 +216,7 @@ const TextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(function Tex
name={name}
onBlur={onBlur}
onChange={onChange}
onClick={onClick}
onFocus={onFocus}
onKeyDown={onKeyDown}
placeholder={placeholder}
Expand Down Expand Up @@ -290,6 +296,7 @@ const TextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(function Tex
name={name}
onBlur={onBlur}
onChange={onChange}
onClick={onClick}
onFocus={onFocus}
onKeyDown={onKeyDown}
placeholder={placeholder}
Expand Down
19 changes: 10 additions & 9 deletions packages/gestalt/src/TextField/InternalTextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ type Props = {
mobileInputMode?: 'none' | 'text' | 'decimal' | 'numeric';
name?: string;
onBlur?: (arg1: { event: React.FocusEvent<HTMLInputElement>; value: string }) => void;
onClick?: (arg1: { event: React.ChangeEvent<HTMLInputElement>; value: string }) => void;
onClick?: (arg1: { event: React.MouseEvent<HTMLInputElement>; value: string }) => void;
onFocus?: (arg1: { event: React.FocusEvent<HTMLInputElement>; value: string }) => void;
onKeyDown?: (arg1: { event: React.KeyboardEvent<HTMLInputElement>; value: string }) => void;
placeholder?: string;
Expand Down Expand Up @@ -156,7 +156,9 @@ const InternalTextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(func
ref,
) {
// ==== REFS ====
const innerRef = useRef<null | HTMLInputElement | HTMLDivElement>(null);
const innerRef = useRef<HTMLInputElement>(null);
const innerTagsRef = useRef<HTMLDivElement>(null);

// When using both forwardRef and innerRefs, useimperativehandle() allows to externally set focus via the ref prop: textfieldRef.current.focus()
// @ts-expect-error - TS2322 - Type 'HTMLDivElement | HTMLInputElement | null' is not assignable to type 'HTMLInputElement'.
useImperativeHandle(ref, () => innerRef.current);
Expand All @@ -171,7 +173,7 @@ const InternalTextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(func
onBlur?.({ event, value: event.currentTarget.value });
};

const handleClick = (event: React.ChangeEvent<HTMLInputElement>) =>
const handleClick = (event: React.MouseEvent<HTMLInputElement>) =>
onClick?.({ event, value: event.currentTarget.value });

const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
Expand Down Expand Up @@ -232,9 +234,10 @@ const InternalTextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(func

const inputElement = (
<input
ref={tags ? undefined : innerRef}
aria-activedescendant={accessibilityActiveDescendant}
aria-controls={accessibilityControls}
// checking for "focused" is not required by screenreaders but it prevents a11y integration tests to complain about missing label, as aria-describedby seems to shadow label in tests though it's a W3 accepeted pattern https://www.w3.org/TR/WCAG20-TECHS/ARIA1.html
aria-controls={accessibilityControls}
aria-describedby={focused ? ariaDescribedby : undefined}
aria-invalid={hasErrorMessage || hasError ? 'true' : 'false'}
autoComplete={autoComplete}
Expand All @@ -250,19 +253,17 @@ const InternalTextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(func
name={name}
onBlur={handleBlur}
onChange={handleChange}
// @ts-expect-error - TS2322 - Type '(event: React.ChangeEvent<HTMLInputElement>) => void | undefined' is not assignable to type 'MouseEventHandler<HTMLInputElement>'.
onClick={handleClick}
onFocus={handleFocus}
onKeyDown={handleKeyDown}
// type='number' doesn't work on ios safari without a pattern
// https://stackoverflow.com/questions/14447668/input-type-number-is-not-showing-a-number-keypad-on-ios
onKeyDown={handleKeyDown}
pattern={type === 'number' ? '\\d*' : undefined}
placeholder={placeholder}
readOnly={readOnly}
// This config is required to prevent exposing passwords and usernames to spell-checking servers during login processes. More info here: https://www.androidpolice.com/google-chrome-servers-get-passwords-enhanced-spell-check/
readOnly={readOnly}
spellCheck={['email', 'password'].includes(type) ? false : undefined}
step={type === 'number' ? step : undefined}
{...(tags ? {} : { ref: innerRef })}
type={type}
value={value}
/>
Expand All @@ -277,7 +278,7 @@ const InternalTextFieldWithForwardRef = forwardRef<HTMLInputElement, Props>(func

<Box position="relative">
{tags ? (
<div className={styledClasses} {...(tags ? { ref: innerRef } : {})}>
<div ref={innerTagsRef} className={styledClasses}>
{tags.map((tag, tagIndex) => (
<Box
// eslint-disable-next-line react/no-array-index-key
Expand Down

0 comments on commit 233d0b4

Please sign in to comment.