-
Notifications
You must be signed in to change notification settings - Fork 18
BASE: Address Management #2286
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
BASE: Address Management #2286
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
npm warn config production Use WalkthroughThe changes in this pull request encompass multiple files, primarily focusing on enhancing type safety through type imports and modifications to address management components. Key updates include reorganizing type imports, implementing optional chaining in functions, and introducing new components for address management, such as adding, editing, and deleting addresses. Additionally, several components related to address input and display have been created, ensuring a structured and user-friendly interface for managing user addresses. Changes
Possibly related PRs
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 (
|
…date hover effect styling for kabab menu button
- Extract duplicated address input logic into a shared component - Add TypeScript interfaces for improved type safety - Implement flexible validation through props - Maintain compatibility with react-hook-form and Material-UI - Reduce code duplication in address form implementations.
- add address type and action constant definitions - Implement address editing functionality with error handling - add TypeScript type definitions for address operations
- add delete functionality for address entries - move isUploadingData state to parent component - refactor child components to use shared loading state
#2330 resolves this |
…emove-billing-address-option Address mgmt/remove billing address option
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: 7
🧹 Outside diff range and nitpick comments (5)
src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx (1)
39-61
: Add error message translation and improve error handlingThe error handling could be improved by:
- Using translated error messages
- Adding specific error handling for network issues
- Adding retry logic for transient failures
Consider implementing the following improvements:
} catch (error) { - setErrors(handleError(error as APIError)); + const apiError = error as APIError; + setErrors([ + { + message: tAddressManagement('errors.unsetBillingAddress') || apiError.message, + code: apiError.code + } + ]); + // Optionally add retry logic here } finally {src/features/user/Settings/EditProfile/AddressManagment/index.tsx (2)
52-65
: Add error retry mechanism for address fetchingThe
updateUserAddresses
function should include a retry mechanism for transient failures.const updateUserAddresses = useCallback(async () => { if (!user || !token || !contextLoaded) return; + const MAX_RETRIES = 3; + let retries = 0; + + const fetchWithRetry = async (): Promise<Address[] | null> => { + try { + return await getAuthenticatedRequest<Address[]>( + tenantConfig.id, + '/app/addresses', + token, + logoutUser + ); + } catch (error) { + if (retries < MAX_RETRIES && isRetryableError(error)) { + retries++; + await new Promise(resolve => setTimeout(resolve, 1000 * retries)); + return fetchWithRetry(); + } + throw error; + } + }; try { - const res = await getAuthenticatedRequest<Address[]>( - tenantConfig.id, - '/app/addresses', - token, - logoutUser - ); + const res = await fetchWithRetry(); if (res) setUserAddresses(res); } catch (error) { setErrors(handleError(error as APIError)); } }, [user, token, contextLoaded, tenantConfig.id, logoutUser]);
80-149
: Consider extracting modal content mapping to a separate componentThe
renderModalContent
logic is complex and could be extracted to improve maintainability.Consider creating a separate
AddressActionModal
component to handle the modal content rendering based on the action type. This would:
- Reduce the complexity of the main component
- Make the code more maintainable
- Improve testability
- Make it easier to add new action types in the future
Example structure:
interface AddressActionModalProps { action: AddressAction; props: { setIsModalOpen: SetState<boolean>; setAddressAction: SetState<AddressAction | null>; selectedAddressForAction?: Address | null; updateUserAddresses?: () => Promise<void>; // ... other props }; } const AddressActionModal: React.FC<AddressActionModalProps> = ...src/utils/addressManagement.ts (2)
53-58
: Consider handling the 'other' address type infindAddressByType
functionThe
findAddressByType
function currently handles only 'primary' and 'mailing' address types. SinceADDRESS_TYPE
includes 'other', consider updating the function to handle the 'other' type or clarify if this exclusion is intentional.Modify the function to accept all address types:
export const findAddressByType = ( addresses: Address[], - addressType: 'primary' | 'mailing' + addressType: keyof typeof ADDRESS_TYPE ) => { return addresses.find((address) => address.type === addressType); };
87-98
: Use appropriate logging methods for error handlingCurrently, errors are logged using
console.log(error);
. It's recommended to useconsole.error(error);
for logging errors, which aids in distinguishing error messages from general logs.Apply this diff to update the error logging:
// In suggestAddress function } catch (error) { - console.log(error); + console.error(error); return []; } // In fetchAddressDetails function } catch (error) { - console.log(error); + console.error(error); return null; }Also applies to: 122-138
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
public/static/locales/en/editProfile.json
is excluded by!**/*.json
📒 Files selected for processing (4)
src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagment/index.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagment/microComponents/AddressActionMenu.tsx
(1 hunks)src/utils/addressManagement.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/features/user/Settings/EditProfile/AddressManagment/index.tsx
[error] 183-183: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
src/utils/addressManagement.ts (4)
34-46
: Approved: getFormattedAddress
function is well-implemented
The getFormattedAddress
function correctly formats address components, handling undefined and null values appropriately.
82-101
: Approved: suggestAddress
function logic is correct
The suggestAddress
function effectively retrieves address suggestions and handles errors by returning an empty array when necessary.
114-138
: Approved: fetchAddressDetails
function retrieves address details accurately
The fetchAddressDetails
function properly fetches detailed address information and gracefully handles cases when no candidates are found.
60-67
: Verify initialization of geocoder
without credentials
When initializing the geocoder
, if process.env.ESRI_CLIENT_SECRET
is not set, an empty options object {}
is used. Ensure that the geocoder-arcgis
library functions correctly without credentials and that the application handles missing or invalid credentials gracefully.
Run the following script to check the behavior when credentials are missing:
src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagment/UnsetBillingAddress.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagment/microComponents/AddressActionMenu.tsx
Outdated
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagment/microComponents/AddressActionMenu.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagment/index.tsx
Outdated
Show resolved
Hide resolved
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.
Functionality looks good.
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: 15
🧹 Outside diff range and nitpick comments (24)
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss (5)
9-17
: Consider enhancing the modal-container mixin for better reusability and compatibility.The modal-container mixin could be improved in several ways:
@mixin modal-container { position: absolute; left: 50%; top: 50%; - transform: translate(-50%, -50%); + transform: translate(-50%, -50%); + -webkit-transform: translate(-50%, -50%); padding: 20px; border-radius: 16px; - background: rgba(255, 255, 255, 1); + background: #ffffff; + z-index: 1000; // Ensure modal appears above other content }
101-120
: Consider responsive design for address actions menu.The fixed
min-width: 184px
might cause overflow issues on smaller screens. Consider using a relative width or adding a mobile breakpoint..addressActions { - min-width: 184px; + min-width: min(184px, 50vw); cursor: pointer; padding: 2px 12px; list-style: none; }
128-148
: Improve responsive design approach for modal forms.Instead of relying on breakpoint overrides, consider using a more fluid approach from the start:
.addressFormLayout { @include flex-container(column); @include modal-container; - min-width: 565px; + width: min(565px, 95vw); + max-height: 90vh; + overflow-y: auto; gap: 40px; }
177-200
: Improve organization of mobile styles.Consider using CSS custom properties for repeated values and consolidating mobile styles:
+:root { + --mobile-container-width: 95%; + --mobile-gap: 8px; +} @include xsPhoneView { .addressFormLayout { min-width: fit-content; - width: 95%; + width: var(--mobile-container-width); } .addressActionContainer { - width: 95%; + width: var(--mobile-container-width); min-width: fit-content; .buttonContainer { flex-direction: column; - gap: 8px; + gap: var(--mobile-gap); } } }
203-205
: Enhance spinner positioning.Consider adding alignment properties for better centering:
.addressMgmtSpinner { @include flex-container(row, center); + align-items: center; + min-height: 100px; // Provide space for spinner }src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1)
154-155
: UseuserAddresses.length
to determine if the address list should be rendered.Currently,
shouldRenderAddressList
relies onuser?.addresses
, but sinceuserAddresses
is your state variable that may have been updated independently, it's more consistent to useuserAddresses.length > 0
.Apply this diff to update the condition:
-const shouldRenderAddressList = - user?.addresses !== undefined && user.addresses.length > 0; +const shouldRenderAddressList = userAddresses.length > 0;src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx (1)
79-79
: Remove unnecessarygeocoder
dependency fromuseCallback
hooks.The
geocoder
variable is imported but not directly used within thehandleSuggestAddress
andhandleGetAddress
functions. Including it in the dependency array is unnecessary and could cause unwanted re-renders.Apply this diff to update the dependency arrays:
// For handleSuggestAddress - [geocoder, country] + [country] // For handleGetAddress - [geocoder, setValue] + [setValue]Also applies to: 102-102
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressDetails.tsx (1)
16-19
: Ensuretype
is a valid class name before using it inclassName
.Using
type
directly inclassName={styles[type]}
could pose risks iftype
contains unexpected values. To prevent potential issues, consider validatingtype
against a list of expected values or mapping it to a specific class name.Example modification:
+ const validTypes = ['primary', 'mailing', 'other']; + const addressTypeClass = validTypes.includes(type) ? styles[type] : ''; return ( <div className={styles.addressDetails}> {type !== 'other' && ( - <span className={styles[type]}> + <span className={addressTypeClass}> {tAddressManagement(`addressType.${type}`)} </span> )} <FormattedAddressBlock userAddress={userAddress} /> </div> );src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx (1)
5-9
: Consider adding loading and disabled states to Props interfaceThe form buttons could benefit from additional states to handle form submission:
- Loading state for the submit button
- Disabled state for both buttons during submission
interface Props { text: string; handleSubmit: () => void; handleCancel: () => void; + isSubmitting?: boolean; }
src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx (1)
23-23
: Improve null/empty string handling for address2The current null check for address2 doesn't handle empty strings. Consider using a more robust check.
- {address2 !== null && <p>{address2}</p>} + {address2 && address2.trim() !== '' && <p>{address2}</p>}src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx (1)
8-13
: Consider adding loading state to Props interfaceThe component could benefit from a loading state to show a skeleton loader while fetching addresses.
interface Props { addresses: Address[]; setAddressAction: SetState<AddressAction | null>; setSelectedAddressForAction: SetState<Address | null>; setIsModalOpen: SetState<boolean>; + isLoading?: boolean; }
src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx (1)
17-36
: Consider memoizing the component for performance optimizationThe component is well-structured but could benefit from memoization since it receives multiple callback functions as props.
-const SingleAddress = ({ +const SingleAddress = React.memo(({ userAddress, addressCount, setIsModalOpen, setAddressAction, setSelectedAddressForAction, -}: Props) => { +}: Props) => { return ( <div className={styles.singleAddressContainer}> <AddressDetails userAddress={userAddress} /> <AddressActionsMenu addressCount={addressCount} setAddressAction={setAddressAction} setIsModalOpen={setIsModalOpen} setSelectedAddressForAction={setSelectedAddressForAction} userAddress={userAddress} /> </div> ); -}; +});src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx (2)
6-9
: Consider restricting the index signature typeThe
[key: string]: any
index signature is too permissive and could lead to type safety issues.interface AddressOption { text: string; - [key: string]: any; + [key: string]: string | number | boolean | undefined; }
63-72
: Consider debouncing the onInputChange callbackFor better performance and to avoid excessive API calls, consider debouncing the input change handler.
+import { useCallback } from 'react'; +import debounce from 'lodash/debounce'; + const debouncedInputChange = useCallback( + debounce((value: string) => { + onInputChange?.(value); + }, 300), + [onInputChange] + ); onInputChange={(_, newValue) => { field.onChange(newValue); - onInputChange?.(newValue); + debouncedInputChange(newValue); }}src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx (2)
67-86
: Extract loading spinner to a reusable componentThe loading spinner UI could be extracted to a reusable component since it might be used in other address management actions.
+const LoadingSpinner = () => ( + <div className={styles.addressMgmtSpinner}> + <CircularProgress color="success" /> + </div> +); - {!isLoading ? ( + {isLoading ? ( + <LoadingSpinner /> + ) : ( <div className={styles.buttonContainer}> <WebappButton text={tCommon('cancel')} elementType="button" variant="secondary" onClick={handleCancel} /> <WebappButton text={tAddressManagement('deleteAction.deleteButton')} elementType="button" variant="primary" onClick={deleteAddress} /> </div> - ) : ( - <div className={styles.addressMgmtSpinner}> - <CircularProgress color="success" /> - </div> )}
16-21
: Consider adding data-testid attributes for testingTo improve testability, consider adding data-testid attributes to key elements.
interface Props { setIsModalOpen: SetState<boolean>; addressId: string | undefined; updateUserAddresses: () => Promise<void>; setAddressAction: SetState<AddressAction | null>; + 'data-testid'?: string; }
src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1)
76-95
: Add loading state for initial context loadingThe component should show a loading state while the context is being loaded to prevent potential undefined behavior.
- {!isLoading ? ( + {!contextLoaded ? ( + <div className={styles.addressMgmtSpinner}> + <CircularProgress color="success" /> + </div> + ) : !isLoading ? (src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx (1)
73-86
: Remove unnecessary dependencies from useCallbackThe dependencies array includes utilities that don't need to be dependencies as they don't change between renders.
[ contextLoaded, user, token, country, selectedAddressForAction?.type, selectedAddressForAction?.id, tenantConfig.id, logoutUser, updateUserAddresses, - handleError, - putAuthenticatedRequest, ]src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (2)
73-76
: Remove redundant condition in addAddress functionThe setUserAddresses prop is required according to the Props interface, making the condition check unnecessary.
- if (res && setUserAddresses) { + if (res) { setUserAddresses((prevAddresses) => [...prevAddresses, res]); - } + }
84-96
: Remove unnecessary dependencies from useCallbackSimilar to EditAddress.tsx, the dependencies array includes utilities that don't need to be dependencies.
[ contextLoaded, user, token, country, logoutUser, setUserAddresses, - handleError, - setIsLoading, - setIsModalOpen, - postAuthenticatedRequest, ]src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx (2)
69-81
: Consider extracting translation keys to constantsThe component uses multiple translation keys directly in the JSX. This could make maintenance harder and is prone to typos.
Consider extracting translation keys to constants:
const TRANSLATION_KEYS = { ADDRESS_TYPE: 'addressType', SET_ADDRESS_CONFIRMATION: 'updateAddressType.setAddressConfirmation', REPLACE_ADDRESS_WARNING: 'updateAddressType.replaceAddressWarning' } as const;
87-102
: Consider extracting button componentsThe button section contains duplicated button structure. Consider extracting these into a separate component for better maintainability.
Example refactor:
interface ActionButtonProps { text: string; variant: 'primary' | 'secondary'; onClick: () => void; } const ActionButton: React.FC<ActionButtonProps> = ({ text, variant, onClick }) => ( <WebappButton text={text} elementType="button" variant={variant} onClick={onClick} /> );src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (2)
40-70
: Consider moving action configuration to a separate fileThe
addressActionConfig
array contains business logic that could be reused across components. Consider moving it to a separate configuration file.Create a new file
addressActionConfig.ts
:import { ADDRESS_TYPE, ADDRESS_ACTIONS } from './addressManagement'; import type { AddressActionItem } from './types'; export const getAddressActionConfig = ( t: (key: string) => string, type: string, addressCount: number ): AddressActionItem[] => [ { label: t(`actions.edit`), action: ADDRESS_ACTIONS.EDIT, shouldRender: true, }, // ... rest of the config ];
92-100
: Add tooltip for kebab menu buttonThe kebab menu button could benefit from a tooltip to improve user experience.
Consider adding a tooltip:
+import { Tooltip } from '@mui/material'; +<Tooltip title="Address Actions"> <button onClick={openPopover} className={styles.kebabMenuButton} aria-label="action menu" aria-expanded={open} aria-haspopup="true" > <KebabMenuIcon /> </button> +</Tooltip>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/index.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressDetails.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormLayout.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/index.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/user/Settings/EditProfile/index.tsx
🔇 Additional comments (7)
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss (1)
92-99
: Verify color contrast ratios for address type labels.
Since these labels use white text (#fff
), ensure that $primaryDarkColor
and $mediumBlueColor
provide sufficient contrast ratio (at least 4.5:1) for WCAG compliance.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormLayout.tsx (1)
1-17
: Clean and well-structured component.
The AddressFormLayout
component is straightforward and effectively organizes the form layout. Good use of TypeScript for type safety.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressFormButtons.tsx (1)
11-31
: LGTM! Clean and focused component implementation
The component is well-structured with clear separation of concerns and proper use of translations.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/FormattedAddressBlock.tsx (1)
11-27
: LGTM! Well-structured component with good performance optimization
The component demonstrates good practices:
- Proper null checking for userAddress
- Effective use of useMemo for computed values
- Clean type definitions
src/features/user/Settings/EditProfile/AddressManagement/microComponents/SingleAddress.tsx (1)
9-15
: LGTM! Props interface is well-defined with appropriate types.
The interface clearly defines the expected props with proper typing, making good use of the SetState
type for state management functions.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressInput.tsx (1)
11-24
: LGTM! Props interface is comprehensive and well-typed
Good use of optional properties and specific types for callbacks.
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)
47-49
:
Validate and sanitize default country value
The default country value from stored config should be validated against a list of allowed countries to prevent potential security issues.
src/features/user/Settings/EditProfile/AddressManagement/AddressManagement.module.scss
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/index.tsx
Outdated
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx
Show resolved
Hide resolved
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx
Show resolved
Hide resolved
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: 2
🧹 Outside diff range and nitpick comments (5)
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (2)
40-70
: Consider extracting address action configuration.The
addressActionConfig
array could be moved to a separate configuration file to improve maintainability and reusability.Consider moving this configuration to a separate file like
addressActionConfig.ts
:export const getAddressActionConfig = ( tAddressManagement: (key: string) => string, type: string, addressCount: number ): AddressActionItem[] => [ { label: tAddressManagement(`actions.edit`), action: ADDRESS_ACTIONS.EDIT, shouldRender: true, }, // ... rest of the config ];
80-85
: Consider adding error handling for action clicks.The
handleActionClick
function should handle potential errors that might occur during state updates.const handleActionClick = (action: AddressAction) => { + try { setSelectedAddressForAction(userAddress); setIsModalOpen(true); setAddressAction(action); setPopoverAnchor(null); + } catch (error) { + console.error('Error handling address action:', error); + // Consider showing a user-friendly error message + } };src/features/user/Settings/EditProfile/AddressManagement/index.tsx (3)
34-42
: Consider using reducer pattern for complex state management.The component manages multiple related states (userAddresses, addressAction, selectedAddressForAction). Using useReducer would make the state transitions more predictable and maintainable.
type AddressState = { addresses: Address[]; action: AddressAction | null; selectedAddress: Address | null; isModalOpen: boolean; }; type AddressAction = | { type: 'SET_ADDRESSES'; payload: Address[] } | { type: 'SET_ACTION'; payload: AddressAction | null } | { type: 'SELECT_ADDRESS'; payload: Address | null } | { type: 'TOGGLE_MODAL'; payload: boolean }; const addressReducer = (state: AddressState, action: AddressAction): AddressState => { switch (action.type) { case 'SET_ADDRESSES': return { ...state, addresses: action.payload }; // ... other cases } };
80-157
: Extract modal content rendering logic.The
renderModalContent
switch statement is quite long and could be simplified by extracting the rendering logic for each action type into separate components or functions.const modalContentMap = { [ADDRESS_ACTIONS.ADD]: () => ( <AddAddress setIsModalOpen={setIsModalOpen} setUserAddresses={setUserAddresses} setAddressAction={setAddressAction} /> ), // ... other action handlers }; const renderModalContent = useMemo(() => { if (addressAction === null) return <></>; const renderer = modalContentMap[addressAction]; return renderer ? renderer() : <></>; }, [/* dependencies */]);
190-192
: Add loading state for modal content.The modal should show a loading state while the content is being prepared or when async operations are in progress.
<Modal open={isModalOpen} aria-labelledby="address-action-modal-title"> + {isLoading ? ( + <div className={styles.modalLoading}> + <CircularProgress /> + </div> + ) : ( {renderModalContent} + )} </Modal>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/static/locales/en/editProfile.json
is excluded by!**/*.json
📒 Files selected for processing (8)
src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/index.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx
(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx
- src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressList.tsx
- src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx
- src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx
- src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressForm.tsx
- src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx
🔇 Additional comments (1)
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx (1)
15-19
: LGTM! Well-structured interface definition.
The AddressActionItem
interface clearly defines the contract for action items with appropriate types.
src/features/user/Settings/EditProfile/AddressManagement/microComponents/AddressActionMenu.tsx
Show resolved
Hide resolved
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.
LGTM
Includes:
To do: