-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: new meta manager UI #513
Conversation
WalkthroughWalkthroughThe changes involve updates to the user interface's JSON configuration, focusing on improving labels and descriptions related to API and metadata management. Key modifications include renaming existing labels for clarity, adding new labels for enhanced functionality, and providing context for metadata management. These adjustments aim to streamline user understanding and improve the overall organization of the metadata section. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as User Interface
participant MetaManager
User->>UI: Open metadata management
UI->>MetaManager: Load metadata
MetaManager->>UI: Display metadata fields
User->>UI: Propose new metadata
UI->>MetaManager: Send proposed edits
MetaManager->>UI: Update displayed metadata
User->>UI: Confirm new metadata
UI->>MetaManager: Finalize metadata updates
MetaManager->>UI: Refresh metadata display
Suggested reviewers
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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
useEffect(() => { | ||
if (requiredMeta.federation_icon_url) { | ||
fetch(requiredMeta.federation_icon_url) | ||
.then((response) => { | ||
if (!response.ok) throw new Error('Invalid icon URL'); | ||
return response.blob(); | ||
}) | ||
.then((blob) => { | ||
if (blob.type.startsWith('image/')) { | ||
const objectURL = URL.createObjectURL(blob); | ||
setIconPreview(objectURL); | ||
setIsIconValid(true); | ||
} else { | ||
throw new Error('Invalid image format'); | ||
} | ||
}) | ||
.catch(() => { | ||
setIconPreview(null); | ||
setIsIconValid(false); | ||
}); | ||
} else { | ||
setIconPreview(null); | ||
setIsIconValid(true); | ||
} | ||
}, [requiredMeta.federation_icon_url]); |
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.
Improve error handling and prevent memory leaks.
Consider the following suggestions:
- Provide more specific error messages for different failure scenarios, such as invalid URL, network error, or invalid image format. This will help with debugging and error tracking.
- Revoke the
objectURL
usingURL.revokeObjectURL
when the component unmounts or the icon URL changes to prevent memory leaks.
#506 <- addresses |
57fd133
to
ab60569
Compare
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@sbddesign this is rebased onto the new UI so is good to test now |
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/router/src/guardian-ui/components/dashboard/tabs/meta/MetaManager.tsx (1)
139-168
: LGTM!The
proposeMetaEdits
function logic looks good. It validates the form, prepares the updated metadata, and handles the API submission correctly.Consider improving the error handling by showing a more user-friendly error message instead of the generic "Failed to propose meta edits. Please try again." alert. You could parse the error response and display a specific message based on the error type.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- apps/router/src/guardian-ui/components/dashboard/tabs/FederationTabsCard.tsx (6 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ConfirmNewMetaModal.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/MetaManager.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/NoPendingProposals.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposeMetaModal.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposedMetas.tsx (5 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/RequiredMeta.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ViewConsensusMeta.tsx (3 hunks)
- apps/router/src/guardian-ui/components/setup/screens/connectGuardians/ConnectGuardians.tsx (1 hunks)
- apps/router/src/guardian-ui/utils/index.ts (1 hunks)
- apps/router/src/languages/en.json (1 hunks)
- packages/types/src/meta.ts (1 hunks)
- packages/utils/src/index.tsx (1 hunks)
Additional comments not posted (23)
packages/types/src/meta.ts (1)
22-25
: LGTM!The new
ParsedConsensusMeta
interface provides a more structured and type-safe way to handle parsed metadata values.packages/utils/src/index.tsx (2)
32-37
: LGTM!The
snakeToTitleCase
function logic is correct, and the implementation is accurate.
39-44
: LGTM!The
titleToSnakeCase
function logic is correct, and the implementation is accurate.apps/router/src/guardian-ui/components/dashboard/tabs/meta/ViewConsensusMeta.tsx (2)
7-9
: LGTM!The simplification of the component props is a good change. It makes the component more focused and easier to use.
50-59
: LGTM!The simplified rendering logic is easier to understand and maintain. Good job!
apps/router/src/guardian-ui/components/dashboard/tabs/meta/ConfirmNewMetaModal.tsx (1)
1-78
: LGTM!The
ConfirmNewMetaModal
component is well-structured, follows best practices, and uses appropriate libraries for UI components and translations. The code is readable and maintainable. No issues or improvements identified.apps/router/src/guardian-ui/utils/index.ts (1)
81-88
: LGTM!The
isJsonString
function correctly checks if a string is valid JSON by attempting to parse it and handling any parsing errors. The implementation looks good.apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposeMetaModal.tsx (1)
1-105
: LGTM, no issues found.apps/router/src/guardian-ui/components/setup/screens/connectGuardians/ConnectGuardians.tsx (1)
67-72
: LGTM! The responsiveminW
change looks good.apps/router/src/guardian-ui/components/dashboard/tabs/meta/MetaManager.tsx (4)
1-17
: LGTM!The imports, utility function, and interface changes look good.
41-93
: LGTM!The state management and utility functions look good. The logic is clear and handles the necessary checks.
95-137
: LGTM!The form submission and field handling utility functions are well-structured and the logic is sound. The dynamic custom field functionality is a nice addition.
170-239
: LGTM!The component UI is well-structured and the rendering logic is implemented correctly. The dynamic rendering of optional fields and the conditional rendering of form action buttons based on the form state are handled properly.
apps/router/src/guardian-ui/components/dashboard/tabs/FederationTabsCard.tsx (4)
59-65
: LGTM!The new state variables for managing consensus metadata, submissions, and tab state are necessary for the enhanced functionality.
67-108
: LGTM!The
pollMetaSubmissions
function is a key addition that effectively retrieves and organizes meta submissions from the API. It enables tracking of peer submissions and the current peer's voting status.
120-151
: LGTM!The polling effect for consensus metadata is well-implemented with optimizations to prevent unnecessary updates when there are no changes.
176-215
: LGTM!The UI enhancements with new tabs and badge for pending proposals improve the visibility and user engagement with the metadata management features.
apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposedMetas.tsx (1)
Line range hint
1-336
: LGTM!The refactoring improves the component's clarity, maintainability, and responsiveness. The changes enhance the handling of meta submissions and introduce a more modular approach for user interactions. No major issues or problems are apparent.
apps/router/src/languages/en.json (4)
91-91
: LGTM!
98-99
: LGTM!
102-104
: LGTM!
111-118
: LGTM!apps/router/src/guardian-ui/components/dashboard/tabs/meta/NoPendingProposals.tsx (1)
1-41
: LGTM!The
NoPendingProposals
component is well-implemented and follows best practices. It effectively communicates the absence of pending proposals and guides users to propose new metadata.
setIconPreview(null); | ||
setIsIconValid(false); | ||
if (error instanceof Error) { | ||
console.error(`Icon validation failed: ${error.message}`); | ||
} | ||
} | ||
}; | ||
|
||
if (requiredMeta.federation_icon_url) { | ||
validateIcon(requiredMeta.federation_icon_url); | ||
} else { | ||
setIconPreview(null); | ||
setIsIconValid(true); | ||
} | ||
|
||
return () => { | ||
if (objectURL) { | ||
URL.revokeObjectURL(objectURL); | ||
} | ||
}; | ||
}, [requiredMeta.federation_icon_url]); | ||
|
||
useEffect(() => { | ||
setIsValid(isIconValid); | ||
}, [isIconValid, setIsValid]); | ||
|
||
const handleChange = (key: string, value: string) => { | ||
setRequiredMeta((prev) => ({ ...prev, [key]: value })); | ||
}; | ||
|
||
return ( | ||
<Flex flexDirection='column' gap={4}> | ||
{Object.entries(requiredMeta).map(([key, value]) => ( | ||
<Flex key={key} alignItems='center'> | ||
<Flex flexDirection='column' flex={1}> | ||
<FormLabel fontWeight='bold' mb={1}> | ||
{snakeToTitleCase(key)} | ||
</FormLabel> | ||
<Flex alignItems='center'> | ||
<Input | ||
type={ | ||
key === 'popup_end_timestamp' | ||
? 'datetime-local' | ||
: key === 'federation_icon_url' | ||
? 'url' | ||
: 'text' | ||
} | ||
value={value} | ||
onChange={(e) => handleChange(key, e.target.value)} | ||
borderColor={ | ||
(['federation_name', 'welcome_message'].includes(key) && | ||
value.trim() === '') || | ||
(key === 'federation_icon_url' && !isIconValid) | ||
? 'yellow.400' | ||
: 'inherit' | ||
} | ||
/> | ||
{key === 'federation_icon_url' && ( | ||
<Flex | ||
ml={2} | ||
alignItems='center' | ||
justifyContent='center' | ||
width='36px' | ||
height='36px' | ||
borderRadius='full' | ||
overflow='hidden' | ||
boxShadow='sm' | ||
bg='gray.100' | ||
flexShrink={0} | ||
> | ||
{iconPreview ? ( | ||
<Image | ||
src={iconPreview} | ||
alt='Federation Icon' | ||
width='100%' | ||
height='100%' | ||
objectFit='cover' | ||
/> | ||
) : ( | ||
<Flex | ||
width='100%' | ||
height='100%' | ||
alignItems='center' | ||
justifyContent='center' | ||
fontSize='md' | ||
fontWeight='bold' | ||
color='gray.400' | ||
> | ||
? | ||
</Flex> | ||
)} | ||
</Flex> | ||
)} | ||
</Flex> | ||
</Flex> | ||
</Flex> | ||
))} | ||
</Flex> | ||
); | ||
}; |
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.
Consider refactoring the component for better maintainability and reusability.
The RequiredMeta
component is quite large and could benefit from being broken down into smaller, reusable components. For example, you could extract the icon preview and validation logic into a separate component.
Additionally, consider moving the icon URL validation logic into a separate utility function to keep the component focused on rendering and state management.
Lastly, instead of directly updating the parent's validity state, you could consider having the component manage its own validity state internally and expose it through a callback prop. This would make the component more reusable and less coupled to the parent's state.
555f958
to
5e5d20c
Compare
New Meta Manager UI per @sbddesign figma
https://www.figma.com/design/TcAV8bmhcaQAxiZE8hjWY9/Fedimint-UI?node-id=0-1&node-type=CANVAS&t=aeKskxAmbQmahHrz-0
TODO: Still can't come up with a good view on mobile, @sbddesign can you see if you can come up with something? just switch your UI to mobile and click through, it's hard to get the table looking correct
Summary by CodeRabbit
New Features
Improvements
User Interface Updates