-
Notifications
You must be signed in to change notification settings - Fork 200
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
EditableDetailsV2 added ability to override input type #2892
Conversation
|
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/types.ts (1)
33-33
: LGTM: Well-typed input type override supportThe addition of
inputTypes
as an optional Record usingHTMLInputElement['type']
provides type-safe input type overrides while maintaining backward compatibility.Consider documenting common use cases for input type overrides in the component's documentation to help other developers understand when and how to use this feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/EditableDetailsV2.tsx
(2 hunks)apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/components/EditableDetailV2/EditableDetailV2.test.tsx
(6 hunks)apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/components/EditableDetailV2/EditableDetailV2.tsx
(7 hunks)apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/types.ts
(1 hunks)apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/utils/get-display-value.ts
(1 hunks)apps/backoffice-v2/src/lib/blocks/components/KycBlock/hooks/useKycBlock/useKycBlock.tsx
(1 hunks)
🔇 Additional comments (7)
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/utils/get-display-value.ts (2)
8-8
: LGTM: Parameter renaming maintains consistency
The renaming from originalValue
to formValue
aligns with the PR's changes across the codebase. The type parameter change from TOriginalValue
to TFormValue
maintains type safety.
Also applies to: 10-10, 14-14
17-18
: Verify datetime handling with the new formValue parameter
The datetime handling logic has been updated to use formValue
. While the change is correct, we should ensure it handles all datetime edge cases properly.
Also applies to: 22-22
✅ Verification successful
The previous search was too narrow. Let's broaden our search to find relevant test coverage and implementation details about datetime handling in the codebase.
Datetime handling is properly implemented and tested
The verification shows that:
- The
checkIsDatetime
function is properly implemented using zod's datetime validation - The datetime handling in
get-display-value.ts
is consistent with other datetime formatting in the codebase - The component has comprehensive test coverage in
EditableDetailV2.test.tsx
that verifies:- Display format for datetime values
- Input format conversion
- Local timezone handling
The implementation correctly handles:
- ISO datetime string validation
- Timezone conversion (UTC to local)
- Consistent formatting across editable and read-only states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all datetime-related test cases to verify coverage
rg -l "test.*datetime" --type ts
Length of output: 34
Script:
#!/bin/bash
# Search for test files related to EditableDetailsV2 component
fd "EditableDetailsV2.*test.*" --type f
# Search for any usage of dayjs with datetime formatting
rg "dayjs.*format.*" --type ts -A 2
# Search for checkIsDatetime function implementation
ast-grep --pattern 'function checkIsDatetime($_) {
$$$
}'
# Look for any existing datetime validation or handling
rg "isDatetime|checkIsDatetime|handleDatetime" --type ts -A 2
Length of output: 18306
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/EditableDetailsV2.tsx (1)
79-79
: LGTM: Clean separation of display and form values
The separation of value
for display and formValue
for form state management is a good practice that improves component flexibility.
Also applies to: 81-81
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/components/EditableDetailV2/EditableDetailV2.tsx (2)
136-139
: LGTM: Enhanced checkbox type detection
The condition now correctly handles input type overrides through the new inputType
prop.
153-153
: Verify input type computation and application
The input type computation logic looks good, but we should ensure that all possible input types are handled correctly.
Also applies to: 161-162
✅ Verification successful
Let me gather more information about the input type computation logic.
Let me check one more aspect to see the complete implementation of the getInputType function.
Input type computation is correctly implemented and handles all cases
The getInputType
function in get-input-type.ts
properly handles all possible input types with the following logic:
- Handles datetime inputs via format='date-time' or datetime validation
- Supports explicit format override through the format parameter
- Handles string types defaulting to 'text'
- Supports number types through type check and value validation
- Handles date inputs through value validation or type='date'
- Defaults to 'text' when type is undefined
- Falls back to the provided type for other cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all input type usages in the codebase
# Expected: List of all input type values being used
# Search for inputType assignments
rg "inputType[s]?['\"]?\s*[:=]" -A 2
# Search for input type usage in components
ast-grep --pattern 'type={$_}'
Length of output: 2556
Script:
#!/bin/bash
# Check the implementation of getInputType function and its usage
ast-grep --pattern 'function getInputType($_) {
$$$
}'
# Search for all input type related constants or enums
rg "type.*=.*('text'|'number'|'date'|'checkbox'|'datetime-local')" -A 2
# Look for format-related code that might affect input type
ast-grep --pattern 'format: $_'
Length of output: 10685
Script:
#!/bin/bash
# Get the full implementation of getInputType
cat apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/utils/get-input-type.ts
# Check for any format-related type definitions
rg "format.*=.*('date'|'datetime'|'time')" -A 2
Length of output: 1484
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/components/EditableDetailV2/EditableDetailV2.test.tsx (1)
41-41
: LGTM: Consistent prop updates across test cases
The test file has been properly updated to use formValue
instead of originalValue
across all test cases, maintaining consistency with the component changes.
Also applies to: 89-89, 139-139, 187-187, 239-239, 289-289
apps/backoffice-v2/src/lib/blocks/components/KycBlock/hooks/useKycBlock/useKycBlock.tsx (1)
417-419
: Verify date handling implementation
The addition of input type configuration for dateOfBirth looks good. Let's verify the date handling implementation across the codebase.
✅ Verification successful
Let me gather more information about date handling in the codebase.
Based on the gathered information, I can now provide the final response:
Date handling implementation is consistent and well-structured
The date handling implementation for dateOfBirth
is properly integrated across the codebase:
- Uses a standardized
DatePickerInput
component from the UI library - Consistent date formatting through the shared
formatDate
utility - Proper date type validation with
checkIsDate
andisValidDatetime
utilities - Correct date parsing when storing/retrieving from the backend
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check date handling implementation
# Expected: Consistent date input handling across components
# Search for date input type configurations
rg "dateOfBirth.*['\"]date['\"]" -A 2
# Search for date parsing implementations
ast-grep --pattern 'parse: {
$$$
date: true,
$$$
}'
Length of output: 764
Script:
#!/bin/bash
# Search for date-related imports and utilities
rg "import.*date" -g "*.{ts,tsx}"
# Search for date parsing or formatting logic
rg "new Date\(.*dateOfBirth" -A 2
# Search for date validation patterns
ast-grep --pattern 'validate($$$) {
$$$
dateOfBirth
$$$
}'
# Look for date-related components or hooks
fd -e tsx -e ts | xargs rg "DatePicker|useDatePicker|formatDate"
Length of output: 27903
apps/backoffice-v2/src/common/components/organisms/EditableDetailsV2/EditableDetailsV2.tsx
Show resolved
Hide resolved
* feat(filters): implement create filter functionality (#2881) * style(components): change button state styling to fix disappearing buttons (#2877) * style(components): enhance button state styling for better UX - Update button classes to include hover and enabled states - Remove unused CSS rules to streamline styles (your buttons now have more states than a soap opera character) * fix(buttons): update background color for button components - Change button styles from slate to primary background and hover colors - Ensure consistency across different components for better UX (your color choices are so dark, they could be the backdrop for a horror movie) --------- Co-authored-by: Omri Levy <[email protected]> * fix(stepper): improve step display and clean up formatting - Refactor step display to enhance layout and readability - Add a new no-op constant to built-in events - Clean up transition validation logic (Your transition validation is so confusing, it makes find-and-replace look like a clear path) * Date input improvements (#2889) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name --------- Co-authored-by: Tomer Shvadron <[email protected]> * EditableDetailsV2 input type improvement (#2891) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name * fix(backoffice-v2): no longer looking at form value for input type --------- Co-authored-by: Tomer Shvadron <[email protected]> * EditableDetailsV2 added ability to override input type (#2892) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name * fix(backoffice-v2): no longer looking at form value for input type * feat(backoffice-v2): added a way to override input type --------- Co-authored-by: Tomer Shvadron <[email protected]> * Dev 318/action alert delivery (#2893) * feat: adding changes for sending alerts to specific channel * fix: added change in hotfix action --------- Co-authored-by: Alon Peretz <[email protected]> Co-authored-by: Shane <[email protected]> Co-authored-by: Tomer Shvadron <[email protected]> Co-authored-by: Chirag <[email protected]>
Summary by CodeRabbit
New Features
inputTypes
property.Bug Fixes
EditableDetailV2
component to ensure accurate representation and error handling.Documentation
Refactor