Datetime formats - app ui #1607
Conversation
# Conflicts: # packages/components/package-lock.json # packages/components/package.json
# Conflicts: # packages/components/package.json
| ### version 5.X | ||
| *Released*: X October 2024 | ||
| - Date/Time formatting simplifications - Settings UI Changes | ||
| - TODO |
There was a problem hiding this comment.
reminder to add this info before merging
| const { onChange } = this.props; | ||
|
|
||
| let value = evt.target.value; | ||
| export const getInitDateTimeSetting = ( |
There was a problem hiding this comment.
I only see one usage of this function in this file. Does it need to be exported?
There was a problem hiding this comment.
I was planning to add jest test for it
| isDate: boolean, | ||
| isTime: boolean, |
There was a problem hiding this comment.
it looks like the usages for this is using the type: 'dateTime' | 'date' | 'time' type to determine these booleans. Could that type just be passed through instead and used directly?
| case DateFormatType.DateTime: | ||
| settingName = 'Date Times'; | ||
| parentFormat = formats.dateTimeFormat; | ||
| currentFormat = inherited ? parentFormat : fieldFormat; |
There was a problem hiding this comment.
minor: it looks like all three of the cases set this the same. it could be moved to a const above the switch
There was a problem hiding this comment.
Because each depend on the actual value of parentFormat , they cannot be moved out of the switch.
| inherited: checked, | ||
| }; | ||
| if (checked) { | ||
| const parentFormat = prevSetting.parentFormat; |
There was a problem hiding this comment.
I like this behavior in the app where you see the parent format when you check to use default. This is different then the LKS behavior that just disables the inputs but keeps their current value (which may or may not match the parent format).
| (name: string, selectedValue: string, selectedOption: SelectInputOption): void => { | ||
| onFormatChange(selectedOption?.value, true); | ||
| }, | ||
| [domainFieldId, onChange] |
There was a problem hiding this comment.
same question here about the domainFieldId usage in the dependency array
| <div className="col-xs-9"> | ||
| <input | ||
| checked={setting.inherited} | ||
| onChange={onToggleInherited} |
There was a problem hiding this comment.
in quick testing locally, it looks like changing the use default checkbox state isn't enough to put the page into a dirty state. maybe that is intentional
| } | ||
|
|
||
| export function splitDateTimeFormat(dateTimeFormat: string): string[] { | ||
| if (dateTimeFormat.indexOf(' h') > -0 || dateTimeFormat.indexOf(' H') > -0) { |
There was a problem hiding this comment.
why the negative 0 in the check?
There was a problem hiding this comment.
wow that must been added by lint
|
|
||
| export function isStandardDateTimeDisplayFormat(dateTimeFormat: string): boolean { | ||
| if (!dateTimeFormat) return false; | ||
| const parts = splitDateTimeFormat(dateTimeFormat.trim()); |
There was a problem hiding this comment.
should the trim() here just move into the splitDateTimeFormat?
| To control how a {contentLabel} value is displayed, provide a string format compatible with the Java{' '} | ||
| <JavaDocsLink urlSuffix="java/text/SimpleDateFormat.html">SimpleDateFormat</JavaDocsLink> class. |
There was a problem hiding this comment.
as mentioned in chat, we should update this as the user isn't "providing" a string (more like "selecting"). Also, no need to refer to the JavaDocs anymore so can probably drop that part.
# Conflicts: # packages/components/package-lock.json # packages/components/package.json # packages/components/releaseNotes/components.md
# Conflicts: # biologics/package-lock.json # biologics/package.json # inventory/package-lock.json # inventory/package.json # sampleManagement/package-lock.json # sampleManagement/package.json
# Conflicts: # packages/components/package-lock.json # packages/components/package.json # packages/components/releaseNotes/components.md
# Conflicts: # packages/components/package-lock.json # packages/components/package.json # packages/components/releaseNotes/components.md # packages/components/src/index.ts
Rationale
Support choosing from a set of standard date/time formats for Application Setting and domain fields.
Related Pull Requests
Changes