Added Date time picker component in react vanilla components#175
Added Date time picker component in react vanilla components#175armaang1729 wants to merge 1 commit into
Conversation
armaang1729
force-pushed
the
dateTimeInput
branch
from
4a69e6b to
4915c86
Compare
iamsudhanshu
left a comment
iamsudhanshu
left a comment
There was a problem hiding this comment.
Thanks for adding this! The component is in good shape overall — value normalization for datetime-local, min/max, disabled, and aria wiring are all handled, with broad test coverage. A few items worth addressing (only #1 is blocking):
1. Test failure — wrong BEM class in the "tooltip and description toggle" test
__tests__/components/DateTimeInput.test.tsx:156
The test looks up cmp-adaptiveform-datetimeinput__questionmark, but the component passes bemBlock='cmp-adaptiveform-datetime', so Label renders cmp-adaptiveform-datetime__questionmark. The lookup returns an empty collection, button[0] is undefined, and userEvent.click(undefined) throws — this test will fail in CI. Change the class to cmp-adaptiveform-datetime__questionmark.
2. DOM diverges from the Core datetime.html — missing __input-wrapper
src/components/DateTimeInput.tsx:72
The Core Date Time component wraps the widget in <div class="cmp-adaptiveform-datetime__input-wrapper">, but here the <input> is a direct child of FieldWrapper. Since the intent is parity with the Core datetime component's BEM/DOM, please wrap the input in __input-wrapper so the headless DOM matches the Core markup/CSS contract.
3. data-cmp-is casing mismatch
src/components/DateTimeInput.tsx:57
It's set to data-cmp-is="adaptiveFormdatetime", but the Core component (datetime.html and the clientlib static IS) uses adaptiveFormDatetime (capital D).
4. Stray demo mapping key
src/utils/mappings.ts:51
'forms-components-examples/components/form/datetime': DateTimeInput maps an examples/demo resourceType rather than the Core resourceType (core/fd/components/form/datetime/v1/datetime). This looks like local-testing leftover — please remove it or replace it with the Core resourceType.
5. Unrelated change in a shared file
src/utils/withRuleEngine.tsx
The enumNames/EnumName typing refactor is behavior-preserving but unrelated to the datetime component, and there's no accompanying af-core version bump in this PR. Consider splitting it into its own PR to keep this change focused and avoid churn in a shared file.
| @@ -0,0 +1,98 @@ | |||
| // ******************************************************************************* | |||
| // * Copyright 2023 Adobe | |||
| const enumNames = state?.enumNames || []; | ||
| return enumNames.map((item: EnumName | string, index: number) => { | ||
| const EnumName = typeof item === 'object' ? item.value : item; | ||
| return enumNames.map((item: EnumNameItem, index: number) => { |
There was a problem hiding this comment.
Why was this changed for date time??
| 'number-input': NumberField, | ||
| 'date-input': DateInput, | ||
| 'datetime-input': DateTimeInput, | ||
| 'forms-components-examples/components/form/datetime': DateTimeInput, |
There was a problem hiding this comment.
forms-components-examples/components/form/datetime' will not come. remove it
| import sanitizeHTML from 'sanitize-html'; | ||
| import { FieldViewState } from './type'; | ||
|
|
||
| /** Legacy rich-text enum option shape; af-core 0.22.175+ types enumNames as string[] only. */ |
|
@armaang1729 Kindly incorporate the review comments |
3 participants
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: