Skip to content
New issue

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

#

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

Already on GitHub? # to your account

(fix) O3-3634: DatePicker input should clear when unspecified is checked #354

Closed
wants to merge 4 commits into from
Closed

(fix) O3-3634: DatePicker input should clear when unspecified is checked #354

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28c9a6d
(Fix) DatePicker input should clear when unspecified is checked
ODORA0 Jul 24, 2024
20c6595
use question id as key
ODORA0 Jul 24, 2024
29ba840
Fix build failure
ODORA0 Jul 25, 2024
747edca
Merge branch 'main' into o3-3634
brandones Aug 5, 2024
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
15 changes: 13 additions & 2 deletions src/components/inputs/date/date.component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import { type CalendarDate, getLocalTimeZone } from '@internationalized/date';
const DateField: React.FC<FormFieldProps> = ({ question, onChange, handler, previousValue }) => {
const { t } = useTranslation();
const [field] = useField(question.id);
const [unspecifiedField] = useField(`${question.id}-unspecified`);
const { setFieldValue, encounterContext, layoutType, workspaceLayout, fields } = React.useContext(FormContext);
const [time, setTime] = useState('');
const { errors, setErrors, warnings, setWarnings } = useFieldValidationResults(question);
const [key, setKey] = useState(0);

const isInline = useMemo(() => {
if (['view', 'embedded-view'].includes(encounterContext.sessionMode) || isTrue(question.readonly)) {
Expand Down Expand Up @@ -82,7 +84,15 @@ const DateField: React.FC<FormFieldProps> = ({ question, onChange, handler, prev
}
}, [field.value, time]);

return encounterContext.sessionMode == 'view' || encounterContext.sessionMode == 'embedded-view' ? (
useEffect(() => {
if (unspecifiedField.value) {
setFieldValue(question.id, null);
setTime(null);
setKey(prevKey => prevKey + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use an int as key as increment it?

Copy link
Contributor

Choose a reason for hiding this comment

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

CC @samuelmale

}
}, [unspecifiedField.value]);

return encounterContext.sessionMode === 'view' || encounterContext.sessionMode === 'embedded-view' ? (
<FieldValueView
label={t(question.label)}
value={field.value instanceof Date ? getDisplay(field.value, question.datePickerFormat) : field.value}
Expand All @@ -97,6 +107,7 @@ const DateField: React.FC<FormFieldProps> = ({ question, onChange, handler, prev
<div className={styles.datePickerSpacing}>
<Layer>
<OpenmrsDatePicker
key={key}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the field id as the key instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

@arodidev When using a dynamic key like field.value, the component may not behave as expected in some cases. So using the original approach which also ensure the component updates correctly when unspecified is check works well with tests as well

Copy link
Contributor

Choose a reason for hiding this comment

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

The question id is not dynamic, and would be much more suited rather than the int here

Copy link
Member Author

Choose a reason for hiding this comment

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

The major issue is with how the OpenmrsDatePicker propagates the states from the main component to the DatePickerInput component in esm core, when the value is cleared it never trickles down cc @samuelmale
We can report this bug but doing this as well will also re-render the component thus clearing the input when unspecified is check

Copy link
Member

Choose a reason for hiding this comment

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

@ibacher any thoughts on the above?

Copy link
Member

Choose a reason for hiding this comment

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

This should be fixed.

id={question.id}
onChange={onDateChange}
labelText={
Expand Down Expand Up @@ -159,7 +170,7 @@ const DateField: React.FC<FormFieldProps> = ({ question, onChange, handler, prev

function getDisplay(date: Date, rendering: string) {
const dateString = formatDate(date);
if (rendering == 'both') {
if (rendering === 'both') {
return `${dateString} ${formatTime(date)}`;
}
return dateString;
Expand Down
Loading