Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass through original values for invalid dates #1949

Merged
merged 4 commits into from
Jun 26, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions packages/material/src/controls/MaterialDateControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ import {
LocalizationProvider
} from '@mui/lab';
import AdapterDayjs from '@mui/lab/AdapterDayjs';
import { createOnChangeHandler, getData, useFocus } from '../util';
import {
createOnChangeHandler,
getData,
useFocus,
useParsedDateSynchronizer,
} from '../util';

export const MaterialDateControl = (props: ControlProps)=> {
const [focused, onFocus, onBlur] = useFocus();
Expand Down Expand Up @@ -79,6 +84,7 @@ export const MaterialDateControl = (props: ControlProps)=> {
handleChange,
saveFormat
),[path, handleChange, saveFormat]);
const parsedDateSynchronizer = useParsedDateSynchronizer({ data, onBlur });

return (
<Hidden xsUp={!visible}>
Expand All @@ -103,10 +109,15 @@ export const MaterialDateControl = (props: ControlProps)=> {
autoFocus={appliedUiSchemaOptions.focus}
error={!isValid}
fullWidth={!appliedUiSchemaOptions.trim}
inputProps={{ ...params.inputProps, type: 'text' }}
inputProps={{
...params.inputProps,
type: 'text',
value: parsedDateSynchronizer.value,
onChange: parsedDateSynchronizer.createOnChangeHandler(params.inputProps.onChange),
}}
InputLabelProps={data ? { shrink: true } : undefined}
onFocus={onFocus}
onBlur={onBlur}
onBlur={parsedDateSynchronizer.onBlur}
variant={'standard'}
/>
)}
Expand Down
18 changes: 15 additions & 3 deletions packages/material/src/controls/MaterialDateTimeControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ import {
LocalizationProvider
} from '@mui/lab';
import AdapterDayjs from '@mui/lab/AdapterDayjs';
import { createOnChangeHandler, getData, useFocus } from '../util';
import {
createOnChangeHandler,
getData,
useFocus,
useParsedDateSynchronizer,
} from '../util';

export const MaterialDateTimeControl = (props: ControlProps) => {
const [focused, onFocus, onBlur] = useFocus();
Expand Down Expand Up @@ -82,6 +87,8 @@ export const MaterialDateTimeControl = (props: ControlProps) => {
saveFormat
),[path, handleChange, saveFormat]);

const parsedDateSynchronizer = useParsedDateSynchronizer({ data, onBlur });

return (
<Hidden xsUp={!visible}>
<LocalizationProvider dateAdapter={AdapterDayjs}>
Expand All @@ -106,10 +113,15 @@ export const MaterialDateTimeControl = (props: ControlProps) => {
autoFocus={appliedUiSchemaOptions.focus}
error={!isValid}
fullWidth={!appliedUiSchemaOptions.trim}
inputProps={{ ...params.inputProps, type: 'text' }}
inputProps={{
...params.inputProps,
type: 'text',
value: parsedDateSynchronizer.value,
onChange: parsedDateSynchronizer.createOnChangeHandler(params.inputProps.onChange),
}}
InputLabelProps={data ? { shrink: true } : undefined}
onFocus={onFocus}
onBlur={onBlur}
onBlur={parsedDateSynchronizer.onBlur}
variant={'standard'}
/>
)}
Expand Down
18 changes: 15 additions & 3 deletions packages/material/src/controls/MaterialTimeControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ import {
LocalizationProvider
} from '@mui/lab';
import AdapterDayjs from '@mui/lab/AdapterDayjs';
import { createOnChangeHandler, getData, useFocus } from '../util';
import {
createOnChangeHandler,
getData,
useFocus,
useParsedDateSynchronizer,
} from '../util';

export const MaterialTimeControl = (props: ControlProps) => {
const [focused, onFocus, onBlur] = useFocus();
Expand Down Expand Up @@ -82,6 +87,8 @@ export const MaterialTimeControl = (props: ControlProps) => {
saveFormat
),[path, handleChange, saveFormat]);

const parsedDateSynchronizer = useParsedDateSynchronizer({ data, onBlur });

return (
<Hidden xsUp={!visible}>
<LocalizationProvider dateAdapter={AdapterDayjs}>
Expand All @@ -106,10 +113,15 @@ export const MaterialTimeControl = (props: ControlProps) => {
autoFocus={appliedUiSchemaOptions.focus}
error={!isValid}
fullWidth={!appliedUiSchemaOptions.trim}
inputProps={{ ...params.inputProps, type: 'text' }}
inputProps={{
...params.inputProps,
type: 'text',
value: parsedDateSynchronizer.value,
onChange: parsedDateSynchronizer.createOnChangeHandler(params.inputProps.onChange),
}}
InputLabelProps={data ? { shrink: true } : undefined}
onFocus={onFocus}
onBlur={onBlur}
onBlur={parsedDateSynchronizer.onBlur}
variant={'standard'}
/>
)}
Expand Down
40 changes: 38 additions & 2 deletions packages/material/src/util/datejs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import dayjs from 'dayjs';
import customParsing from 'dayjs/plugin/customParseFormat';
import { useState, useMemo, FormEvent, FormEventHandler } from 'react';

// required for the custom save formats in the date, time and date-time pickers
dayjs.extend(customParsing);
Expand All @@ -8,13 +9,13 @@ export const createOnChangeHandler = (
path: string,
handleChange: (path: string, value: any) => void,
saveFormat: string | undefined
) => (time: dayjs.Dayjs) => {
) => (time: dayjs.Dayjs, textInputValue: string) => {
if (!time) {
handleChange(path, undefined);
return;
}
const result = dayjs(time).format(saveFormat);
handleChange(path, result === 'Invalid Date' ? undefined : result);
handleChange(path, result === 'Invalid Date' ? textInputValue : result);
};

export const getData = (
Expand All @@ -30,3 +31,38 @@ export const getData = (
}
return dayjsData;
};

type DateInputFormEvent = FormEvent<HTMLInputElement | HTMLTextAreaElement>;

/**
* Improves the UX of date fields by controlling the rendered input value.
* When a user enters a date value that ends up being different than the
* the value of `data`, then on blur we sync the rendered input value.
*
* @param data The parsed date value.
* @param onBlur Additional handler to run after input value is sync'd.
* @returns Props to pass to the rendered input element.
*/
export const useParsedDateSynchronizer = (props: {
data: any;
onBlur: FormEventHandler<HTMLInputElement | HTMLTextAreaElement> | undefined;
}) => {
const [value, setValue] = useState(props.data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not take into account cases where the data is changed differently, for example it will not update when data is changed via the pickers themselves or when the user hands in new data when externally updated.


const onBlur = useMemo(
() => (event: DateInputFormEvent) => {
setValue(props.data);
if (props.onBlur) props.onBlur(event);
},
[props.data, props.onBlur]
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use onBlur directly however it's already used for focused. So instead of injecting ourselves into onBlur we can just check the value of focused.


const createOnChangeHandler = (
onChange: (event: DateInputFormEvent) => void
) => (event: DateInputFormEvent) => {
setValue((event.target as HTMLInputElement | HTMLTextAreaElement).value);
if (onChange) onChange(event);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way we completely bypass the current change semantics. It can certainly work but we need to keep all edge cases in mind. It's probably easier to "simply" transform the value handed in by the picker by ignoring it when we're not focused. This also makes it overall simpler as we don't manage another explicit state.


return { value, onBlur, createOnChangeHandler };
};
27 changes: 27 additions & 0 deletions packages/material/test/renderers/MaterialDateControl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,4 +381,31 @@ describe('Material date control', () => {
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('04---1961');
});

it('should call onChange with original input value for invalid date strings', () => {
const core = initCore(schema, uischema);
const onChangeData: any = {
data: undefined
};
wrapper = mount(
<JsonFormsStateProvider initState={{ renderers: materialRenderers, core }}>
<TestEmitter
onChange={({ data }) => {
onChangeData.data = data;
}}
/>
<MaterialDateControl
schema={schema}
uischema={{...uischema}}
/>
</JsonFormsStateProvider>
);

const input = wrapper.find('input').first();
expect(input.props().value).toBe('');

(input.getDOMNode() as HTMLInputElement).value = 'invalid date string';
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('invalid date string');
});
});
27 changes: 27 additions & 0 deletions packages/material/test/renderers/MaterialDateTimeControl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -387,4 +387,31 @@ describe('Material date time control', () => {
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('2005/12/10 11:22 am');
});

it('should call onChange with original input value for invalid date strings', () => {
const core = initCore(schema, uischema);
const onChangeData: any = {
data: undefined
};
wrapper = mount(
<JsonFormsStateProvider initState={{ renderers: materialRenderers, core }}>
<TestEmitter
onChange={({ data }) => {
onChangeData.data = data;
}}
/>
<MaterialDateTimeControl
schema={schema}
uischema={{...uischema}}
/>
</JsonFormsStateProvider>
);

const input = wrapper.find('input').first();
expect(input.props().value).toBe('');

(input.getDOMNode() as HTMLInputElement).value = 'invalid date string';
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('invalid date string');
});
});
27 changes: 27 additions & 0 deletions packages/material/test/renderers/MaterialTimeControl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,4 +381,31 @@ describe('Material time control', () => {
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('1//12 am');
});

it('should call onChange with original input value for invalid date strings', () => {
const core = initCore(schema, uischema);
const onChangeData: any = {
data: undefined
};
wrapper = mount(
<JsonFormsStateProvider initState={{ renderers: materialRenderers, core }}>
<TestEmitter
onChange={({ data }) => {
onChangeData.data = data;
}}
/>
<MaterialTimeControl
schema={schema}
uischema={{...uischema}}
/>
</JsonFormsStateProvider>
);

const input = wrapper.find('input').first();
expect(input.props().value).toBe('');

(input.getDOMNode() as HTMLInputElement).value = 'invalid date string';
input.simulate('change', input);
expect(onChangeData.data.foo).toBe('invalid date string');
});
});