group page rework / modal fixes #127
Conversation
WalkthroughThe changes in this pull request focus on enhancing the modal components across various parts of the application. Key modifications include the introduction of Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (30)
components/groups/modals/updateGroupModal.tsx (2)
316-334: Consider standardizing z-index valuesWhile the high z-index (10000) ensures the modal appears above other elements, it's recommended to:
- Define z-index values in a centralized constants file
- Use relative z-index values based on a standardized stacking context
This helps maintain consistent stacking behavior across the application.
Consider creating a shared constants file:
// constants/zIndex.ts export const Z_INDEX = { MODAL_BACKDROP: 9998, MODAL: 9999, TOAST: 10000, } as const;Then update the modal:
- zIndex: 10000, + zIndex: Z_INDEX.MODAL,
514-519: Enhance modal backdrop accessibilityWhile the backdrop implementation is good, it could benefit from additional accessibility attributes.
Apply this diff to enhance accessibility:
<div className="fixed inset-0 bg-black bg-opacity-50" style={{ zIndex: -1 }} onClick={() => setShowUpdateModal(false)} aria-hidden="true" + role="presentation" + aria-label="Close modal" />components/groups/modals/memberManagementModal.tsx (1)
193-208: Consider moving inline styles to CSS classesWhile the styling effectively addresses the stacking context issues, consider moving the inline styles to CSS classes for better maintainability and consistency.
Example refactor:
- style={{ - position: 'fixed', - top: 0, - left: 0, - right: 0, - bottom: 0, - zIndex: 9999, - backgroundColor: 'transparent', - ... - }} + className="fixed inset-0 z-modal bg-transparent flex items-center justify-center h-screen w-screen"Then define these classes in your CSS/Tailwind config:
.z-modal { z-index: 9999; }Also applies to: 387-395
components/react/views/Contacts.tsx (2)
Line range hint
279-285: Replace direct DOM manipulation with React state management for modal controlThe current implementation uses direct DOM manipulation (
getElementById().showModal()) to control the message edit modal, which:
- Breaks React's declarative paradigm
- Can cause issues with React's virtual DOM synchronization
- Makes testing more difficult
- May contribute to modal stacking context issues mentioned in issue Stacking Context / Modals / Toast Notifs #119
Consider refactoring to use React state management with the Headless UI Dialog component:
- if (showMessageEditModal) { - (document.getElementById('message_edit_modal') as HTMLDialogElement).showModal(); - } + if (showMessageEditModal) { + onMessageEditModalOpen(); // Call a prop function to update modal state + }Add these props to the component:
onMessageEditModalOpen: () => void; // Callback to open the message edit modalThis approach:
- Maintains React's declarative nature
- Ensures proper state management
- Aligns with the solution proposed in issue Stacking Context / Modals / Toast Notifs #119 for fixing modal stacking
- Makes the component more testable
Also applies to: 366-372
Line range hint
1-489: Consider decomposing the Contacts component for better maintainabilityThe component currently handles multiple responsibilities:
- Contact list management
- Contact form handling
- Search functionality
- Import/Export functionality
- Modal interaction
Consider splitting this into smaller, focused components:
ContactList: Handle the list display and searchContactForm: Handle add/edit form logicContactActions: Handle import/export functionalityContactItem: Individual contact display and actionsThis would:
- Improve code maintainability
- Make testing easier
- Reduce cognitive load when making changes
- Allow for better separation of concerns
Would you like me to provide a detailed example of this component breakdown?
components/groups/modals/voteDetailsModal.tsx (5)
469-485: Suggestion: Extract inline styles to a CSS module or styled componentThe inline styles applied to the
dialogelement can be moved to a CSS module or a styled component. This refactor will improve maintainability and keep styling consistent across the application.Apply this diff to refactor the inline styles:
- <dialog - id="vote-details-modal" - className={`modal ${showVoteModal ? 'modal-open' : ''}`} - style={{ - position: 'fixed', - top: 0, - left: 0, - right: 0, - bottom: 0, - zIndex: 9999, - backgroundColor: 'transparent', - padding: 0, - margin: 0, - height: '100vh', - width: '100vw', - display: showVoteModal ? 'flex' : 'none', - alignItems: 'center', - justifyContent: 'center', - }} - > + <dialog + id="vote-details-modal" + className={`modal ${showVoteModal ? 'modal-open' : ''} custom-dialog`} + >Then, define the
.custom-dialogclass in your CSS:.custom-dialog { position: fixed; top: 0; left: 0; right: 0; bottom: 0; z-index: 9999; background-color: transparent; padding: 0; margin: 0; height: 100vh; width: 100vw; display: flex; align-items: center; justify-content: center; }
724-781: Avoid duplication by creating reusable modal componentsThe modal backdrop and close button code in
messages_modalis duplicated. Consider creating a reusable modal component or extracting common elements to adhere to the DRY (Don't Repeat Yourself) principle, improving maintainability.
793-821: Consistent handling of modal backdrop and stylesSimilar to the
messages_modal, the inline styles and backdrop handling for the main modal are duplicated. Refactoring these into reusable components or styles will enhance consistency and make future maintenance easier.
465-485: Accessibility concern: Missingariaattributes on modalTo improve accessibility, consider adding appropriate
ariaattributes to the modal, such asaria-modal="true"andaria-labelledbyoraria-labelfor dialog identification.Apply this diff to add accessibility attributes:
<dialog id="vote-details-modal" + aria-modal="true" + aria-labelledby="vote-modal-title" className={`modal ${showVoteModal ? 'modal-open' : ''} custom-dialog`} >And update the title element:
- <p className="text-xl font-bold text-primary-content"> + <p id="vote-modal-title" className="text-xl font-bold text-primary-content">
736-749: Handle unknown message types gracefullyIn the
messages_modal, if a message type is not found in theimportantFieldsmapping, it defaults to showing only the@typefield. Consider providing a fallback or a user-friendly message for unknown message types to enhance user experience.components/factory/modals/MintModal.tsx (4)
71-85: Consider moving inline styles to CSS classesThe inline styles applied to the
<dialog>element could be moved to CSS classes to improve maintainability and consistency with the rest of the codebase.
67-86: Ensure cross-browser compatibility of the<dialog>elementThe
<dialog>element is not fully supported in all browsers, which may affect users on unsupported browsers. Consider using a more widely supported modal implementation or including a polyfill to ensure consistent behavior across all supported browsers.
82-82: Use CSS classes instead of inline styles for dynamic displayRather than toggling the
displayproperty using inline styles, consider applying CSS classes to control the modal's visibility. This approach enhances readability and aligns with best practices.
89-91: Improve accessibility of the close buttonTo enhance accessibility, add an
aria-labelto the close button so that screen readers can properly announce its purpose.Apply this diff:
<button className="btn btn-sm btn-circle btn-ghost absolute right-2 top-2 text-[#00000099] dark:text-[#FFFFFF99] hover:bg-[#0000000A] dark:hover:bg-[#FFFFFF1A]" + aria-label="Close modal" >components/factory/modals/BurnModal.tsx (1)
48-68: Consider extracting inline styles to CSS classes for maintainabilityThe extensive inline styles in the
<dialog>component may affect code readability and maintainability. It's advisable to move these styles into CSS classes or utilize a styling solution like CSS modules or styled-components for better organization and reusability.components/bank/forms/sendForm.tsx (3)
109-111: Consider removing redundant error handling for wallet connectionThe
handleSendfunction throws an error when the wallet is not connected, but error handling is already managed by theuseTxhook. Additional error handling may be unnecessary.
116-118: Reassess error handling for missing admin address in group transactionsThrowing an error when
adminis not provided may be unnecessary since theuseTxhook handles errors. Alternatively, consider makingadmina required prop whenisGroupis true to ensure type safety.
147-147: Avoid logging sensitive information in productionThe
console.logstatement logs the address, which may contain sensitive information. Consider removing debug logs or ensuring they're appropriately gated for development environments.Apply this diff to remove the console log:
- console.log('Estimating fee for address:', address);components/groups/components/groupControls.tsx (4)
335-336: Clarify time left when voting period has endedWhen the voting period has ended,
timeLeftis set to'none', which might be unclear to users. Consider setting it to'Expired'or'Voting period ended'for better clarity.Apply this diff to update the time left indication:
if (diff <= 0) { - timeLeft = 'none'; + timeLeft = 'Expired';
169-186: ExtracttypeRegistryto a separate constants fileTo improve maintainability and reusability, consider moving the
typeRegistryobject to a separate constants or utilities file. This will make it easier to manage message type mappings across the application.
188-199: EnhancegetHumanReadableTypefunction for better maintainabilityConsider adding comments to the
getHumanReadableTypefunction to explain its logic. Additionally, if this function is used elsewhere, moving it to a utility file could improve reusability.
377-379: Simplify redundant CSS class namesThe
classNameattribute contains redundant class names (xxs:rounded-r-[12px] xs:rounded-r-[12px]). Simplify the class names to avoid redundancy and ensure consistent styling.Apply this diff to clean up the class names:
- className={`bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%] sm:rounded-none xxs:rounded-r-[12px] xs:rounded-r-[12px] xl:rounded-r-none`} + className={`bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%] sm:rounded-none rounded-r-[12px] xl:rounded-r-none`}components/groups/components/myGroups.tsx (1)
175-176: Simplify loading state determinationThe
isLoadingGroupInfovariable checks multiple loading states. If any of the dependencies are loading, the group info should be considered loading. Ensure that this logic accurately reflects the required loading states.components/bank/components/sendBox.tsx (1)
91-91: PassisGroupprop toIbcSendFormonly if supportedThe
IbcSendFormcomponent receives theisGroupprop, but it's not clear if it utilizes this prop. Confirm whetherIbcSendFormsupports group transactions, and if not, avoid passing unused props to maintain clarity.Apply this diff to remove the
isGroupprop if not used:<IbcSendForm ... - isGroup={isGroup} ... />pages/groups/index.tsx (1)
85-85: Consider using a more explicit refetch strategyThe current implementation uses a logical OR (
||) betweenrefetchGroupByMemberandrefetchProposals. While this works, it might be clearer to handle these separately or create a combined refetch function.Consider this alternative approach:
-refetch={refetchGroupByMember || refetchProposals} +refetch={() => { + refetchGroupByMember?.(); + refetchProposals?.(); +}}This makes the intention clearer and ensures both refreshes are attempted when available.
Also applies to: 97-97
components/admins/modals/warningModal.tsx (2)
99-114: Consider extracting inline styles to CSS classesWhile the fixed positioning and z-index values effectively solve the stacking context issues, consider moving these styles to CSS classes for better maintainability and reusability across different modals.
Example CSS class approach:
-style={{ - position: 'fixed', - top: 0, - left: 0, - right: 0, - bottom: 0, - zIndex: 9999, - backgroundColor: 'transparent', - padding: 0, - margin: 0, - height: '100vh', - width: '100vw', - display: openWarningModal ? 'flex' : 'none', - alignItems: 'center', - justifyContent: 'center', -}} +className={`fixed inset-0 z-[9999] p-0 m-0 h-screen w-screen ${ + openWarningModal ? 'flex items-center justify-center' : 'hidden' +}`}Also applies to: 159-167
156-171: Consider removing the visible close button from the backdropThe backdrop form already handles modal closing on click. The visible close button is unnecessary and might be confusing to users.
<form method="dialog" className="modal-backdrop" style={{ position: 'fixed', top: 0, left: 0, right: 0, bottom: 0, zIndex: -1, backgroundColor: 'rgba(0, 0, 0, 0.3)', }} onSubmit={handleClose} > - <button>close</button> </form>components/admins/modals/validatorModal.tsx (1)
153-270: Consider extracting common modal stylesWhile the implementation is correct, there's duplicate styling code across modal components. Consider extracting common modal styles into a shared utility or component.
Example approach:
// utils/modalStyles.ts export const commonModalStyles = { position: 'fixed', top: 0, left: 0, right: 0, bottom: 0, zIndex: 9999, backgroundColor: 'transparent', padding: 0, margin: 0, height: '100vh', width: '100vw', display: 'flex', alignItems: 'center', justifyContent: 'center', } as const;components/bank/components/historyBox.tsx (1)
171-175: Consider using a constant for header text.The header text is duplicated in both desktop and mobile views. Consider extracting it to a constant to improve maintainability.
+const getHeaderText = (isGroup: boolean, isMobile: boolean) => { + if (isMobile) { + return isGroup ? 'Transactions' : 'History'; + } + return isGroup ? 'Group Transactions' : 'Transaction History'; +}; -<h3 className="text-lg md:text-xl font-semibold text-[#161616] dark:text-white hidden xl:block"> - {isGroup ? 'Group Transactions' : 'Transaction History'} -</h3> -<h3 className="text-lg md:text-xl font-semibold text-[#161616] dark:text-white block xl:hidden"> - {isGroup ? 'Transactions' : 'History'} -</h3> +<h3 className="text-lg md:text-xl font-semibold text-[#161616] dark:text-white hidden xl:block"> + {getHeaderText(isGroup ?? false, false)} +</h3> +<h3 className="text-lg md:text-xl font-semibold text-[#161616] dark:text-white block xl:hidden"> + {getHeaderText(isGroup ?? false, true)} +</h3>components/bank/forms/ibcSendForm.tsx (1)
Line range hint
149-152: Consider removing redundant error handling.Based on previous learnings, the
useTxhook already handles errors, making the try-catch block redundant.-try { const exponent = values.selectedToken.metadata?.denom_units[1]?.exponent ?? 6; const amountInBaseUnits = parseNumberToBigInt(values.amount, exponent).toString(); // ... rest of the code -} catch (error) { - console.error('Error during sending:', error); -} finally { - setIsSending(false); -} +const exponent = values.selectedToken.metadata?.denom_units[1]?.exponent ?? 6; +const amountInBaseUnits = parseNumberToBigInt(values.amount, exponent).toString(); +// ... rest of the code +setIsSending(false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (25)
components/admins/modals/validatorModal.tsx(2 hunks)components/admins/modals/warningModal.tsx(4 hunks)components/bank/components/historyBox.tsx(4 hunks)components/bank/components/sendBox.tsx(3 hunks)components/bank/components/tokenList.tsx(3 hunks)components/bank/forms/ibcSendForm.tsx(2 hunks)components/bank/forms/sendForm.tsx(3 hunks)components/bank/modals/sendModal.tsx(5 hunks)components/factory/modals/BurnModal.tsx(3 hunks)components/factory/modals/MintModal.tsx(3 hunks)components/factory/modals/updateDenomMetadata.tsx(4 hunks)components/groups/components/__tests__/groupProposals.test.tsx(1 hunks)components/groups/components/groupControls.tsx(1 hunks)components/groups/components/groupProposals.tsx(0 hunks)components/groups/components/index.tsx(1 hunks)components/groups/components/myGroups.tsx(9 hunks)components/groups/modals/groupInfo.tsx(5 hunks)components/groups/modals/memberManagementModal.tsx(4 hunks)components/groups/modals/updateGroupModal.tsx(4 hunks)components/groups/modals/voteDetailsModal.tsx(5 hunks)components/react/views/Contacts.tsx(1 hunks)next.config.js(0 hunks)pages/bank.tsx(4 hunks)pages/groups/index.tsx(2 hunks)tests/mock.ts(4 hunks)
💤 Files with no reviewable changes (2)
- next.config.js
- components/groups/components/groupProposals.tsx
🧰 Additional context used
📓 Learnings (2)
components/bank/forms/ibcSendForm.tsx (1)
Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/bank/forms/ibcSendForm.tsx:149-152
Timestamp: 2024-11-12T13:03:18.927Z
Learning: In `components/bank/forms/ibcSendForm.tsx`, the `handleSend` function relies on the `useTx` hook for error handling, so additional error handling or user notifications within `handleSend` are unnecessary.
components/bank/forms/sendForm.tsx (1)
Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/bank/forms/ibcSendForm.tsx:149-152
Timestamp: 2024-11-12T13:03:18.927Z
Learning: In `components/bank/forms/ibcSendForm.tsx`, the `handleSend` function relies on the `useTx` hook for error handling, so additional error handling or user notifications within `handleSend` are unnecessary.
🔇 Additional comments (31)
components/groups/modals/updateGroupModal.tsx (3)
15-15: LGTM: Props changes align with modal improvements
The addition of visibility control props and createPortal import aligns with best practices for modal implementations, addressing the stacking context issues mentioned in issue #119.
Also applies to: 22-23, 29-30
304-314: LGTM: Well-implemented keyboard accessibility
The escape key handling is properly implemented with:
- Event cleanup in useEffect
- Event propagation control
- Correct dependency array
526-531: LGTM: Well-implemented portal with SSR safety
The portal implementation follows best practices:
- Safely handles server-side rendering
- Efficiently manages modal rendering
- Properly uses createPortal for optimal stacking context
components/groups/modals/memberManagementModal.tsx (4)
28-29: LGTM: Props addition aligns with modal visibility control requirements
The new props provide explicit control over modal visibility, which is essential for managing the modal's stacking context.
43-52: LGTM: Robust modal visibility control implementation
Good implementation of keyboard accessibility and proper cleanup of event listeners.
403-408: LGTM: Proper portal implementation with SSR handling
The portal implementation correctly handles both client and server-side rendering scenarios.
189-397: Verify stacking context fix with toast notifications
The portal implementation and z-index management should resolve the stacking context issues with toast notifications. Please verify this fix with the following test cases:
- Open modal and trigger a toast notification
- Verify toast appears above the modal
- Test with multiple modals open
✅ Verification successful
Based on the shell script results, I can now generate the final response:
✅ Stacking context implementation is correctly implemented
The implementation has proper z-index management and portal usage:
- Toast notifications are rendered at the highest z-index (100000) via CSS
- All modals, including the member management modal, use
createPortalto render at document.body level - Toast notifications are also rendered using portals with proper layering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other toast notification implementations to ensure consistent z-index
rg -g '*.{tsx,jsx,css,scss}' -A 2 'toast|notification'
# Find all modal implementations to verify consistent portal usage
ast-grep --pattern 'createPortal($$$)'
Length of output: 9916
components/groups/modals/voteDetailsModal.tsx (3)
36-47: Good practice: Defining explicit props interface
Introducing the VoteDetailsModalProps interface enhances type safety and improves code readability by clearly defining the component's expected props.
452-463: Proper handling of Escape key to close the modal
The use of useEffect to add and clean up the keydown event listener ensures that pressing the Escape key closes the modal appropriately. This enhances user experience and prevents memory leaks by properly removing the event listener on unmount.
754-780: Inconsistent modal closing logic
The modal backdrop's onClick handler is calling setShowVoteModal(false); and onClose();, which may cause unexpected behavior if onClose also modifies the showVoteModal state. Ensure that the modal closing logic is consistent and does not cause side effects.
Please verify if the onClose function modifies showVoteModal. If it does, calling both setShowVoteModal(false); and onClose(); might be redundant or cause unintended state updates.
components/factory/modals/BurnModal.tsx (2)
2-2: Appropriate import of createPortal for modal rendering
The import of createPortal from 'react-dom' is correct and aligns with the use of portals to render the modal into the DOM body, addressing stacking context issues.
123-125: Server-side rendering check is appropriate
The condition if (typeof document !== 'undefined') correctly ensures that createPortal is called only in a browser environment, preventing errors during server-side rendering.
components/groups/components/index.tsx (1)
3-3: Ensure consistent exports after refactoring
The export statement has been updated to export * from './groupControls';. Verify that all components previously exported from groupProposals are now correctly exported from groupControls to prevent import errors in other parts of the application.
components/bank/modals/sendModal.tsx (3)
54-74: Excellent implementation of modal stacking context fixes!
The changes effectively address the stacking context issues by:
- Using fixed positioning for both modal and backdrop
- Setting explicit z-index values (9999 for modal, -1 for backdrop)
- Implementing proper backdrop styling with semi-transparent background
This implementation aligns perfectly with the objectives from issue #119.
Also applies to: 103-115
121-126: Great use of createPortal with SSR safety check!
The implementation correctly:
- Checks for browser environment before using createPortal
- Renders modal directly into document.body
- Returns null for server-side rendering
This approach ensures proper modal rendering while maintaining SSR compatibility.
17-19: Verify group-related props usage
New props added for group functionality:
isGroup: Indicates if the modal is used in a group contextadmin: Group admin addressrefetchProposals: Callback to refresh proposals
✅ Verification successful
Let me verify how these props are actually used in the components to ensure proper implementation.
Let me check the implementation of the group-related functionality to ensure these props are properly used.
Based on the comprehensive analysis of the code, I can now provide the final response:
Group functionality properly implemented
The group-related props (isGroup, admin, refetchProposals) are correctly implemented and used throughout the component hierarchy:
- Props are properly passed down through components
- Group transactions are handled via
submitProposalwhenisGroupis true - Admin address is properly validated and used for group transactions
- UI elements adapt to show group-specific labels
- Proper refresh of proposals after successful transactions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the usage of these props in related components
ast-grep --pattern 'interface $_ {
$$$
isGroup?: boolean;
admin?: string;
refetchProposals?: () => void;
$$$
}'
Length of output: 1783
Script:
#!/bin/bash
# Check the usage of these props in the components
rg "isGroup|admin|refetchProposals" components/bank/ -A 3 -B 3
Length of output: 12059
Script:
#!/bin/bash
# Check for group-related message construction and handling
ast-grep --pattern 'submitProposal({
$$$
})'
Length of output: 7624
components/groups/components/__tests__/groupProposals.test.tsx (1)
4-4: Verify test coverage after component relocation
The import path has changed from 'groupProposals' to 'groupControls', suggesting component reorganization.
✅ Verification successful
Import path change is correct and component functionality is preserved
The component has been successfully moved from 'groupProposals' to 'groupControls' with all its functionality intact. The test file's import path update is correct as:
- The component implementation in
groupControls.tsxmatches the test expectations - No remnants of the old location were found
- All test cases in
groupProposals.test.tsxalign with the component's current implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the component structure in groupControls matches the test expectations
ast-grep --pattern 'export $_ function ProposalsForPolicy($_) {
$$$
}'
# Check if there are any other test files that might need similar updates
fd -e test.tsx -e test.ts | xargs rg "from.*groupProposals'"
Length of output: 140
Script:
#!/bin/bash
# Let's verify the component existence and structure in both locations
rg -l "ProposalsForPolicy" --type ts --type tsx
# Check the actual component implementation
ast-grep --pattern 'export $_ ProposalsForPolicy'
# Check test file content to understand test expectations
cat "components/groups/components/__tests__/groupProposals.test.tsx"
# Look for any related test files
fd "group.*test" -e tsx -e ts
Length of output: 4648
Script:
#!/bin/bash
# Let's check the actual implementation in groupControls
cat "components/groups/components/groupControls.tsx"
# Also check if there are any remnants of the old location
fd "groupProposals.tsx" --type f
Length of output: 18453
components/admins/modals/warningModal.tsx (2)
7-7: LGTM! Import changes align with portal-based modal rendering
The addition of createPortal and removal of unused imports improve code cleanliness.
Also applies to: 9-9
175-180: LGTM! Proper portal implementation for modal rendering
The portal implementation correctly handles:
- Browser environment check
- Direct rendering to document.body
- Clean fallback for non-browser environments
pages/bank.tsx (1)
39-40: LGTM! Enhanced loading experience with configurable skeleton counts
The addition of skeleton loading configuration improves the user experience during data fetching, with proper responsive handling for mobile and desktop views.
Also applies to: 160-160, 174-175
components/factory/modals/updateDenomMetadata.tsx (1)
116-135: LGTM! Consistent modal rendering implementation
The portal and styling implementation matches the pattern used in other modals, ensuring consistent behavior across the application.
components/groups/modals/groupInfo.tsx (2)
150-169: LGTM: Modal implementation follows best practices
The modal implementation correctly uses createPortal and proper z-indexing to resolve stacking context issues. The styling ensures the modal is properly centered and covers the viewport.
226-239: LGTM: Proper modal backdrop implementation
The backdrop implementation with semi-transparent background and proper z-indexing ensures the modal appears above other content while maintaining a clear visual hierarchy.
components/bank/components/tokenList.tsx (3)
18-21: LGTM: Clean interface extension for group functionality
The new props cleanly extend the TokenList component to support group functionality while maintaining backward compatibility with optional props.
64-64: LGTM: Clear conditional rendering for group context
The conditional rendering clearly distinguishes between group and personal assets views.
244-246: LGTM: Proper prop forwarding to SendModal
Group-related props are correctly forwarded to the SendModal component.
components/admins/modals/validatorModal.tsx (2)
125-145: LGTM: Proper portal implementation for modal
The modal correctly uses createPortal and maintains consistent styling with other modals in the application.
277-291: LGTM: Proper backdrop implementation
The backdrop implementation is consistent with other modals and properly handles click events.
components/bank/components/historyBox.tsx (2)
Line range hint 56-71: LGTM! Props interface is well-defined.
The new props are properly typed and documented:
skeletonGroupCount: number- Controls number of skeleton groups during loadingskeletonTxCount: number- Controls number of transactions per group during loadingisGroup?: boolean- Optional flag to differentiate group vs. regular transactions
Line range hint 236-254: LGTM! Skeleton loading implementation is well-structured.
The skeleton loading UI provides a good user experience with:
- Proper aria-label for accessibility
- Consistent styling with the actual content
- Dynamic number of skeletons based on props
components/bank/forms/ibcSendForm.tsx (1)
34-34: LGTM! Optional isGroup prop is properly typed.
The isGroup prop is correctly added as an optional boolean parameter.
Also applies to: 47-47
| setFieldValue(fieldName, selectedAddress); | ||
| } | ||
| setIsContactsOpen(false); | ||
| (document.getElementById(modalId) as HTMLDialogElement)?.close(); |
There was a problem hiding this comment.
Remove direct DOM manipulation of dialog element
The direct manipulation of the dialog element using getElementById and close() should be avoided. Instead, use the visibility prop.
Replace the dialog manipulation with state management:
- (document.getElementById(modalId) as HTMLDialogElement)?.close();
+ setShowMemberManagementModal(false);Also applies to: 320-321
|
|
||
| showMessageEditModal = false, | ||
| }: { | ||
| onClose: () => void; | ||
| onReturn?: () => void; | ||
| selectionMode?: boolean; | ||
| onSelect?: (address: string) => void; | ||
| currentAddress?: string; | ||
| showMemberManagementModal?: boolean; | ||
|
|
||
| showMessageEditModal?: boolean; |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent modal prop usage detected across components
The verification reveals that both props (showMemberManagementModal and showMessageEditModal) are actively used in different parts of the codebase, indicating an incomplete migration:
showMemberManagementModalis still used in group-related components:components/groups/components/myGroups.tsxcomponents/groups/modals/memberManagementModal.tsxcomponents/react/modal.tsx
showMessageEditModalis used in:components/groups/forms/proposals/ProposalMessages.tsxcomponents/react/modal.tsxcomponents/react/views/Contacts.tsx
This suggests that the modal management strategy change is only partially implemented, which could lead to inconsistent behavior and potential modal stacking issues.
🔗 Analysis chain
Verify prop changes across dependent components
The replacement of showMemberManagementModal with showMessageEditModal suggests a change in modal management strategy. Let's ensure this change is consistently applied across all dependent components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of the old prop name
rg "showMemberManagementModal"
# Search for components using the new prop name to verify consistent implementation
rg "showMessageEditModal"
Length of output: 1826
| </div> | ||
| <form | ||
| method="dialog" | ||
| className="modal-backdrop" | ||
| style={{ | ||
| position: 'fixed', | ||
| top: 0, | ||
| left: 0, | ||
| right: 0, | ||
| bottom: 0, | ||
| zIndex: -1, | ||
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | ||
| }} | ||
| onSubmit={onClose} | ||
| > | ||
| <button>close</button> | ||
| </form> |
There was a problem hiding this comment.
Adjust z-index of the modal backdrop
Setting zIndex: -1 on the modal backdrop may cause it to appear behind other content, rendering it ineffective. The backdrop should have a higher z-index than the content it overlays to ensure it blocks interactions with underlying elements.
Apply this diff to fix the z-index:
- zIndex: -1,
+ zIndex: 9998,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| </div> | |
| <form | |
| method="dialog" | |
| className="modal-backdrop" | |
| style={{ | |
| position: 'fixed', | |
| top: 0, | |
| left: 0, | |
| right: 0, | |
| bottom: 0, | |
| zIndex: -1, | |
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | |
| }} | |
| onSubmit={onClose} | |
| > | |
| <button>close</button> | |
| </form> | |
| </div> | |
| <form | |
| method="dialog" | |
| className="modal-backdrop" | |
| style={{ | |
| position: 'fixed', | |
| top: 0, | |
| left: 0, | |
| right: 0, | |
| bottom: 0, | |
| zIndex: 9998, | |
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | |
| }} | |
| onSubmit={onClose} | |
| > | |
| <button>close</button> | |
| </form> |
| method="dialog" | ||
| className="modal-backdrop" | ||
| style={{ | ||
| position: 'fixed', | ||
| top: 0, | ||
| left: 0, | ||
| right: 0, | ||
| bottom: 0, | ||
| zIndex: -1, | ||
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | ||
| }} | ||
| onSubmit={onClose} | ||
| > | ||
| <button>close</button> | ||
| </form> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the modal backdrop and closing mechanism
Using a <form> with method="dialog" and an onSubmit handler for the backdrop may be unnecessarily complex. Consider replacing it with a <div> and handling the onClick event to close the modal when the backdrop is clicked. This approach simplifies the DOM structure and improves event handling.
Apply this diff to streamline the backdrop implementation:
-<form
- method="dialog"
- className="modal-backdrop"
- style={{
- position: 'fixed',
- top: 0,
- left: 0,
- right: 0,
- bottom: 0,
- zIndex: -1,
- backgroundColor: 'rgba(0, 0, 0, 0.3)',
- }}
- onSubmit={onClose}
->
- <button>close</button>
-</form>
+<div
+ className="modal-backdrop"
+ style={{
+ position: 'fixed',
+ top: 0,
+ left: 0,
+ right: 0,
+ bottom: 0,
+ backgroundColor: 'rgba(0, 0, 0, 0.3)',
+ }}
+ onClick={onClose}
+/>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| method="dialog" | |
| className="modal-backdrop" | |
| style={{ | |
| position: 'fixed', | |
| top: 0, | |
| left: 0, | |
| right: 0, | |
| bottom: 0, | |
| zIndex: -1, | |
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | |
| }} | |
| onSubmit={onClose} | |
| > | |
| <button>close</button> | |
| </form> | |
| <div | |
| className="modal-backdrop" | |
| style={{ | |
| position: 'fixed', | |
| top: 0, | |
| left: 0, | |
| right: 0, | |
| bottom: 0, | |
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | |
| }} | |
| onClick={onClose} | |
| /> |
| let status = 'Pending'; | ||
| if (proposal.status.toString() === 'PROPOSAL_STATUS_ACCEPTED') { | ||
| status = 'Execute'; | ||
| } else if (proposal.status.toString() === 'PROPOSAL_STATUS_CLOSED') { | ||
| status = 'Executed'; | ||
| } else if (proposalTally) { | ||
| const { isPassing, isThresholdReached, isTie } = | ||
| isProposalPassing(proposalTally); | ||
| if (isThresholdReached) { | ||
| if (isTie) { | ||
| status = 'Tie'; | ||
| } else { | ||
| status = isPassing ? 'Passing' : 'Failing'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Correct proposal status mapping for accurate display
The current logic for determining the proposal status may not accurately reflect the actual proposal states. For example, when proposal.status is 'PROPOSAL_STATUS_ACCEPTED', the displayed status is set to 'Execute', which may be misleading. Similarly, 'PROPOSAL_STATUS_CLOSED' is shown as 'Executed'. Consider revising the status mapping to accurately represent each proposal status.
Apply this diff to adjust the status mapping:
let status = 'Pending';
if (proposal.status.toString() === 'PROPOSAL_STATUS_ACCEPTED') {
- status = 'Execute';
+ status = 'Accepted';
} else if (proposal.status.toString() === 'PROPOSAL_STATUS_CLOSED') {
- status = 'Executed';
+ status = 'Closed';
} else if (proposalTally) {
const { isPassing, isThresholdReached, isTie } =
isProposalPassing(proposalTally);
if (isThresholdReached) {
if (isTie) {
status = 'Tie';
} else {
status = isPassing ? 'Passing' : 'Failing';
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let status = 'Pending'; | |
| if (proposal.status.toString() === 'PROPOSAL_STATUS_ACCEPTED') { | |
| status = 'Execute'; | |
| } else if (proposal.status.toString() === 'PROPOSAL_STATUS_CLOSED') { | |
| status = 'Executed'; | |
| } else if (proposalTally) { | |
| const { isPassing, isThresholdReached, isTie } = | |
| isProposalPassing(proposalTally); | |
| if (isThresholdReached) { | |
| if (isTie) { | |
| status = 'Tie'; | |
| } else { | |
| status = isPassing ? 'Passing' : 'Failing'; | |
| } | |
| } | |
| let status = 'Pending'; | |
| if (proposal.status.toString() === 'PROPOSAL_STATUS_ACCEPTED') { | |
| status = 'Accepted'; | |
| } else if (proposal.status.toString() === 'PROPOSAL_STATUS_CLOSED') { | |
| status = 'Closed'; | |
| } else if (proposalTally) { | |
| const { isPassing, isThresholdReached, isTie } = | |
| isProposalPassing(proposalTally); | |
| if (isThresholdReached) { | |
| if (isTie) { | |
| status = 'Tie'; | |
| } else { | |
| status = isPassing ? 'Passing' : 'Failing'; | |
| } | |
| } |
| isGroup={isGroup} | ||
| admin={admin} | ||
| refetchProposals={refetchProposals} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure required props are provided when isGroup is true
When isGroup is true, admin and refetchProposals may be required by SendForm. Consider updating prop types to reflect this requirement or add validation to prevent unexpected behavior.
| <form | ||
| method="dialog" | ||
| className="modal-backdrop" | ||
| style={{ | ||
| position: 'fixed', | ||
| top: 0, | ||
| left: 0, | ||
| right: 0, | ||
| bottom: 0, | ||
| zIndex: -1, | ||
| backgroundColor: 'rgba(0, 0, 0, 0.3)', | ||
| }} | ||
| > | ||
| <button onClick={() => handleCloseModal()}>close</button> | ||
| </form> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling form submission more gracefully
The backdrop form's close button might interfere with the main form's submission flow. Consider preventing form submission on backdrop click when the form is being submitted.
<form
method="dialog"
className="modal-backdrop"
style={{
position: 'fixed',
top: 0,
left: 0,
right: 0,
bottom: 0,
zIndex: -1,
backgroundColor: 'rgba(0, 0, 0, 0.3)',
}}
+ onSubmit={(e) => {
+ if (isSigning) {
+ e.preventDefault();
+ return;
+ }
+ handleCloseModal();
+ }}
>
- <button onClick={() => handleCloseModal()}>close</button>
+ <button type="submit">close</button>
</form>Committable suggestion skipped: line range outside the PR's diff.
| // @ts-ignore | ||
| '@type': '/cosmos.bank.v1beta1.MsgSend', | ||
| $typeUrl: '/cosmos.bank.v1beta1.MsgSend', | ||
| type_url: '/cosmos.bank.v1beta1.MsgSend', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant type properties.
The code has multiple properties representing the same type information:
@type$typeUrltype_url
This redundancy could lead to inconsistencies. Consider using only one standard property.
-// @ts-ignore
-'@type': '/cosmos.bank.v1beta1.MsgSend',
-$typeUrl: '/cosmos.bank.v1beta1.MsgSend',
-type_url: '/cosmos.bank.v1beta1.MsgSend',
+type_url: '/cosmos.bank.v1beta1.MsgSend',Also applies to: 519-522
| // @ts-ignore | ||
| name: 'Member 1', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore and properly type the member interface.
Instead of using @ts-ignore, extend the MemberSDKType interface to include the name property.
+interface ExtendedMemberSDKType extends MemberSDKType {
+ name: string;
+}
-// @ts-ignore
-name: 'Member 1',
+name: 'Member 1' as ExtendedMemberSDKType['name'],Also applies to: 608-609
| // @ts-ignore | ||
| members: mockMembers, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore and properly type the members property.
Similar to the previous comment, use proper typing instead of @ts-ignore.
+interface ExtendedFormData extends FormData {
+ members: ExtendedMemberSDKType[];
+}
-// @ts-ignore
-members: mockMembers,
+members: mockMembers as ExtendedMemberSDKType[],Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
==========================================
+ Coverage 60.23% 61.13% +0.90%
==========================================
Files 148 148
Lines 14806 15324 +518
==========================================
+ Hits 8919 9369 +450
- Misses 5887 5955 +68 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
components/groups/modals/__tests__/voteDetailsModal.test.tsx (1)
Modal portal rendering tests need enhancement
The test file doesn't explicitly verify the modal's portal rendering in document.body. While the functional tests cover the modal's content and interactions, they should be enhanced to ensure proper portal implementation:
- Add test to verify modal renders in document.body
- Verify modal cleanup on unmount
- Consider testing z-index/stacking context
🔗 Analysis chain
Line range hint
42-93: Verify modal rendering in document body.Since the component now uses
createPortalto render the modal directly in the document body (as mentioned in the AI summary), we should verify this behavior.Run the following script to check the modal implementation:
Let me verify the test coverage for modal portal rendering.
Let me check the test utilities to understand how the modal rendering is handled in tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify modal implementation using createPortal # Look for createPortal usage and modal container setup # Check for createPortal import and usage rg "createPortal" "components/groups/modals/voteDetailsModal.tsx" # Check for modal container setup rg -A 5 "document\.body" "components/groups/modals/voteDetailsModal.tsx"Length of output: 309
Script:
#!/bin/bash # Check test setup and assertions related to modal rendering rg -A 3 "renderWithChainProvider|render" "components/groups/modals/__tests__/voteDetailsModal.test.tsx" # Check test utils implementation fd "renderWithChainProvider" --exec rg -l "render|createPortal"Length of output: 2489
Script:
#!/bin/bash # Check the test utilities implementation rg -A 5 "renderWithChainProvider" "tests/render.ts" "tests/render.tsx" # Check if there's any custom container setup for portal testing rg -A 5 "container" "tests/render.ts" "tests/render.tsx"Length of output: 615
components/groups/modals/__tests__/UpdateGroupModal.test.tsx (1)
Line range hint
56-65: Refactor modal opening mechanismThe manual modal opening using
showModal()is redundant since the component now accepts ashowUpdateModalprop. Consider removing this manual DOM manipulation and rely on the prop-based visibility control instead.beforeEach(() => { renderWithChainProvider(<UpdateGroupModal {...mockProps} />); - - // Programmatically open the modal - const modal = document.getElementById('test-modal') as HTMLDialogElement; - if (modal) { - modal.showModal(); - } });components/bank/components/__tests__/tokenList.test.tsx (3)
64-73: Consider adding test coverage for new props.While the basic rendering test is maintained, the new props (
refetchBalances,refetchHistory) are added as mock functions but their behavior isn't verified. Consider adding test cases to verify:
- That these functions are called when expected
- The impact of different
pageSizevalues on paginationExample addition:
test('calls refetchBalances when triggered', () => { const mockRefetch = jest.fn(); renderWithChainProvider( <TokenList balances={mockBalances} isLoading={false} refetchBalances={mockRefetch} refetchHistory={jest.fn()} address={''} pageSize={1} /> ); // Trigger the refetch (implementation dependent) expect(mockRefetch).toHaveBeenCalled(); });
124-133: Consider adding visual regression tests for modal stacking.Given that this PR addresses modal stacking context issues (per PR #119), consider adding visual regression tests to prevent future regressions. While the current unit tests verify modal content, they don't catch z-index or stacking context issues.
Consider tools like:
- Cypress for visual testing
- Percy or Chromatic for visual regression testing
- CSS-in-JS solutions that can enforce z-index hierarchies
71-71: Consider standardizing pageSize across tests.The
pageSizeprop varies between tests (1 vs 2) without clear reasoning. Consider:
- Standardizing the value across tests unless testing pagination specifically
- Adding a dedicated pagination test suite
- Using a constant for the default page size
Example:
const DEFAULT_PAGE_SIZE = 2; // Use in tests pageSize={DEFAULT_PAGE_SIZE}Also applies to: 154-154
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
components/bank/components/__tests__/tokenList.test.tsx(4 hunks)components/groups/modals/__tests__/UpdateGroupModal.test.tsx(2 hunks)components/groups/modals/__tests__/voteDetailsModal.test.tsx(2 hunks)
🔇 Additional comments (4)
components/groups/modals/__tests__/voteDetailsModal.test.tsx (1)
7-7: LGTM! Clean import organization.
Good job consolidating the mock imports and removing unused SDK type imports. This improves code maintainability.
components/groups/modals/__tests__/UpdateGroupModal.test.tsx (1)
1-1: Verify testing framework configuration
The import of jest from bun:test is unusual since Bun has its own test runner. This might lead to compatibility issues or unexpected behavior.
Also applies to: 3-4
✅ Verification successful
Using bun:test is correct and consistent across the codebase
The import of jest from bun:test is actually correct. The results show that bun:test is used consistently across all test files in the project (36 files). Bun's test runner includes Jest-compatible APIs, which explains why jest is imported from bun:test. This is a valid and intended usage pattern when testing with Bun.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the project uses Bun's test runner consistently
rg -l 'from (\'|")bun:test(\'|")'
# Check for potential mixing of Jest and Bun test configurations
fd -e json -e js -e ts -e tsx -x rg -l 'jest\.config|jest\.setup'
Length of output: 217
Script:
#!/bin/bash
# Check if the project uses Bun's test runner consistently
rg -l "from ['\"]bun:test['\"]"
# Check for Jest configuration files
fd "jest.config" "jest.setup"
# Check for other test imports in test files
fd -e test.tsx -e test.ts -x rg -l "import.*from.*['\"]@testing-library|vitest|jest['\"]"
Length of output: 4755
components/bank/components/__tests__/tokenList.test.tsx (2)
5-5: LGTM! Import statement updated correctly.
The change from default import to named import aligns with the component's export modifications.
5-5: Verify the TokenList component export.
Let's ensure the component file exists and exports correctly.
✅ Verification successful
TokenList component export is correctly implemented
The component exists at the expected path and is properly exported as a named function component. The import in the test file matches the export implementation, and there are no conflicting default exports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TokenList component exists and exports correctly
# Check if the file exists and contains named export
rg -l "export.*TokenList" "components/bank/components/tokenList.tsx"
# Check for any remaining default exports that might conflict
rg "export default" "components/bank/components/tokenList.tsx"
Length of output: 172
Script:
#!/bin/bash
# Let's verify the actual export by checking the content of the file
rg -A 5 "export.*TokenList" "components/bank/components/tokenList.tsx"
# Also check if there are any imports of this component elsewhere to confirm usage
rg "import.*TokenList.*from.*bank/components/tokenList" --type typescript
Length of output: 282
Script:
#!/bin/bash
# Let's verify the actual export by checking the content of the file
rg -A 5 "export.*TokenList" "components/bank/components/tokenList.tsx"
# Also check if there are any imports of this component elsewhere
rg "import.*TokenList.*from.*bank/components/tokenList"
Length of output: 353
| showVoteModal: true, | ||
| setShowVoteModal: jest.fn(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test coverage for modal visibility controls.
While the new props showVoteModal and setShowVoteModal are correctly added to defaultProps, consider adding tests to verify:
- Modal visibility based on
showVoteModalprop - Modal closing behavior using
setShowVoteModalandonClose
Example test cases to add:
test('shows modal when showVoteModal is true', () => {
renderWithChainProvider(<VoteDetailsModal {...defaultProps} showVoteModal={true} />);
expect(screen.getByLabelText('vote-details-modal')).toBeVisible();
});
test('hides modal when showVoteModal is false', () => {
renderWithChainProvider(<VoteDetailsModal {...defaultProps} showVoteModal={false} />);
expect(screen.queryByLabelText('vote-details-modal')).not.toBeInTheDocument();
});
test('calls setShowVoteModal and onClose when closing modal', () => {
renderWithChainProvider(<VoteDetailsModal {...defaultProps} />);
fireEvent.click(screen.getByLabelText('close-modal'));
expect(defaultProps.setShowVoteModal).toHaveBeenCalledWith(false);
expect(defaultProps.onClose).toHaveBeenCalled();
});| showUpdateModal: true, | ||
| setShowUpdateModal: jest.fn(), | ||
| onUpdate: jest.fn(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test cases for modal visibility controls
While the mock props for modal visibility control have been added (showUpdateModal, setShowUpdateModal, onUpdate), there are no test cases verifying their functionality. Consider adding tests to verify:
- Modal shows/hides based on
showUpdateModal setShowUpdateModalis called when closing the modalonUpdateis called with correct parameters when submitting changes
Example test cases to add:
test('modal visibility controls work correctly', () => {
const { rerender } = renderWithChainProvider(<UpdateGroupModal {...mockProps} />);
// Test initial visibility
expect(screen.getByRole('dialog')).toBeVisible();
// Test hiding modal
rerender(<UpdateGroupModal {...mockProps} showUpdateModal={false} />);
expect(screen.queryByRole('dialog')).not.toBeVisible();
});
test('callbacks are triggered correctly', async () => {
renderWithChainProvider(<UpdateGroupModal {...mockProps} />);
// Test modal close
fireEvent.click(screen.getByRole('button', { name: /close/i }));
expect(mockProps.setShowUpdateModal).toHaveBeenCalledWith(false);
// Test update submission
fireEvent.click(screen.getByRole('button', { name: /update/i }));
expect(mockProps.onUpdate).toHaveBeenCalled();
});
This PR reworks the group page and adds tx history and the asset list per group. It also fixes the stacking context issues between modals and toast notifications
adds #55
fixes #119
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor
Style
Tests