-
Notifications
You must be signed in to change notification settings - Fork 40
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
refactor:Single "Fedimint UI" App #522
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughWalkthroughThe changes introduce a new context for managing application state related to guardians and gateways. The implementation includes a context provider for state management, a user interface for connecting to services, and improved configuration handling within the Changes
Possibly related PRs
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: 3
Outside diff range and nitpick comments (4)
packages/ui/tsconfig.json (1)
4-4
: Remove the redundant file from theinclude
array.Including the specific file
src/Header.tsx
in theinclude
array is redundant when the entiresrc
directory is already included. It's best to keep theinclude
array minimal and only specify directories.Apply this diff to remove the redundant file:
{ "extends": "@fedimint/tsconfig/react-library.json", "exclude": ["dist", "node_modules"], - "include": ["src", "src/Header.tsx"] + "include": ["src"] }apps/router/src/context/AppContext.tsx (1)
1-147
: Consider improving error handling, type safety, and reducer conciseness.
Error handling (lines 33-35): Instead of just logging the error to the console, consider displaying a user-friendly error message or falling back to the default initial state.
Type safety (lines 24-43): The
makeInitialState
function could return a more specific type, such asOmit<AppContextValue, 'dispatch'>
, to ensure that thedispatch
function is not null when the initial state is created.Reducer (lines 113-117, 122-126): The reducer could be more concise by using object spread syntax instead of
Object.fromEntries
andObject.entries
. For example:case APP_ACTION_TYPE.REMOVE_GUARDIAN: const { [action.payload]: _, ...remainingGuardians } = state.guardians; return { ...state, guardians: remainingGuardians, };apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsx (2)
30-30
: Ensure consistent naming and typing forGuardianServerStatus
.The
GuardianServerStatus
type is imported from@fedimint/types
. Ensure that this type is consistently used throughout the codebase and matches the naming convention of other similar types.
39-43
: Ensure consistent naming and usage of context hooks.The component uses the
useGuardianSetupApi
anduseGuardianSetupContext
hooks. Ensure that these hooks are consistently named and used throughout the guardian setup flow. Consider adding documentation or comments to clarify their purpose and usage.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (41)
apps/guardian-ui/public/favicon.ico
is excluded by!**/*.ico
apps/router/public/favicon.ico
is excluded by!**/*.ico
apps/router/src/gateway-ui/assets/svgs/check-circle.svg
is excluded by!**/*.svg
apps/router/src/gateway-ui/assets/svgs/copy.svg
is excluded by!**/*.svg
apps/router/src/gateway-ui/assets/svgs/info-solid.svg
is excluded by!**/*.svg
apps/router/src/gateway-ui/assets/svgs/linkIcon.svg
is excluded by!**/*.svg
apps/router/src/gateway-ui/assets/svgs/x-circle.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/images/Fedimint-Full.png
is excluded by!**/*.png
apps/router/src/guardian-ui/assets/svgs/account.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/arrow-left.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/arrow-right.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/banknotes.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/bitcoin-white.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/bitcoin.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/calendar.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/check-circle.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/check.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/chevron-down.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/chevron-up.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/copy.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/ecash.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/fedimint.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/guardians.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/home.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/info.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/intersect-square.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/lightbulb.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/lightningIcon.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/linkIcon.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/modules.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/plus.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/qr.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/settings.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/solo.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/stars.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/trash.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/warning.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/white-check.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/x-circle.svg
is excluded by!**/*.svg
apps/router/src/guardian-ui/assets/svgs/x.svg
is excluded by!**/*.svg
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (101)
- apps/gateway-ui/public/index.html (0 hunks)
- apps/gateway-ui/public/manifest.json (0 hunks)
- apps/gateway-ui/src/ApiProvider.tsx (0 hunks)
- apps/gateway-ui/src/App.tsx (0 hunks)
- apps/gateway-ui/src/index.tsx (0 hunks)
- apps/gateway-ui/tsconfig.json (0 hunks)
- apps/guardian-ui/.eslintrc.js (0 hunks)
- apps/guardian-ui/package.json (0 hunks)
- apps/guardian-ui/public/config.json.example (0 hunks)
- apps/guardian-ui/public/robots.txt (0 hunks)
- apps/guardian-ui/src/App.tsx (0 hunks)
- apps/guardian-ui/src/AppContext.tsx (0 hunks)
- apps/guardian-ui/src/admin/AdminContext.tsx (0 hunks)
- apps/guardian-ui/src/components/NotConfigured.tsx (0 hunks)
- apps/guardian-ui/src/hooks/index.tsx (0 hunks)
- apps/guardian-ui/src/index.tsx (0 hunks)
- apps/guardian-ui/src/languages/ca.json (0 hunks)
- apps/guardian-ui/src/languages/de.json (0 hunks)
- apps/guardian-ui/src/languages/es.json (0 hunks)
- apps/guardian-ui/src/languages/fr.json (0 hunks)
- apps/guardian-ui/src/languages/hu.json (0 hunks)
- apps/guardian-ui/src/languages/it.json (0 hunks)
- apps/guardian-ui/src/languages/ja.json (0 hunks)
- apps/guardian-ui/src/languages/ko.json (0 hunks)
- apps/guardian-ui/src/languages/pt.json (0 hunks)
- apps/guardian-ui/src/languages/ru.json (0 hunks)
- apps/guardian-ui/src/languages/zh.json (0 hunks)
- apps/guardian-ui/src/setup/SetupContext.tsx (0 hunks)
- apps/guardian-ui/tsconfig.json (0 hunks)
- apps/router/package.json (2 hunks)
- apps/router/public/config.json (1 hunks)
- apps/router/public/index.html (1 hunks)
- apps/router/src/context/AppContext.tsx (1 hunks)
- apps/router/src/context/gateway/GatewayContext.tsx (1 hunks)
- apps/router/src/context/guardian/AdminContext.tsx (1 hunks)
- apps/router/src/context/guardian/GuardianContext.tsx (1 hunks)
- apps/router/src/context/guardian/SetupContext.tsx (1 hunks)
- apps/router/src/context/hooks.tsx (1 hunks)
- apps/router/src/gateway-ui/Gateway.tsx (1 hunks)
- apps/router/src/gateway-ui/GatewayApi.ts (1 hunks)
- apps/router/src/gateway-ui/components/ConnectFederationModal.tsx (2 hunks)
- apps/router/src/gateway-ui/components/HeaderWithUnitSelector.tsx (2 hunks)
- apps/router/src/gateway-ui/components/federations/ConnectFederation.tsx (3 hunks)
- apps/router/src/gateway-ui/components/federations/FederationsTable.tsx (1 hunks)
- apps/router/src/gateway-ui/components/walletCard/WalletCard.tsx (1 hunks)
- apps/router/src/gateway-ui/components/walletModal/index.tsx (2 hunks)
- apps/router/src/gateway-ui/components/walletModal/receive/ReceiveEcash.tsx (2 hunks)
- apps/router/src/gateway-ui/components/walletModal/receive/ReceiveLightning.tsx (4 hunks)
- apps/router/src/gateway-ui/components/walletModal/receive/ReceiveOnchain.tsx (4 hunks)
- apps/router/src/gateway-ui/components/walletModal/send/SendEcash.tsx (3 hunks)
- apps/router/src/gateway-ui/components/walletModal/send/SendOnchain.tsx (4 hunks)
- apps/router/src/gateway-ui/types.ts (1 hunks)
- apps/router/src/guardian-ui/Guardian.tsx (1 hunks)
- apps/router/src/guardian-ui/GuardianApi.ts (5 hunks)
- apps/router/src/guardian-ui/admin/FederationAdmin.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceCard.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/FederationInfoCard.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/InviteCode.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/danger/DownloadBackup.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/danger/GuardianAuthenticationCode.tsx (5 hunks)
- apps/router/src/guardian-ui/components/dashboard/danger/SignApiAnnouncement.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/gateways/GatewaysCard.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/MetaManager.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposedMetas.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/meta/ViewConsensusMeta.tsx (2 hunks)
- apps/router/src/guardian-ui/components/setup/RunDKG.tsx (3 hunks)
- apps/router/src/guardian-ui/components/setup/screens/connectGuardians/ConnectGuardians.tsx (4 hunks)
- apps/router/src/guardian-ui/components/setup/screens/roleSelector/RoleSelector.tsx (2 hunks)
- apps/router/src/guardian-ui/components/setup/screens/setConfiguration/SetConfiguration.tsx (3 hunks)
- apps/router/src/guardian-ui/components/setup/screens/setupComplete/SetupComplete.tsx (1 hunks)
- apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsx (4 hunks)
- apps/router/src/guardian-ui/hooks/index.tsx (1 hunks)
- apps/router/src/guardian-ui/setup/FederationSetup.tsx (4 hunks)
- apps/router/src/guardian-ui/types.tsx (1 hunks)
- apps/router/src/home/HomePage.tsx (1 hunks)
- apps/router/src/index.tsx (1 hunks)
- apps/router/src/languages/ca.json (1 hunks)
- apps/router/src/languages/de.json (1 hunks)
- apps/router/src/languages/en.json (2 hunks)
- apps/router/src/languages/es.json (1 hunks)
- apps/router/src/languages/fr.json (1 hunks)
- apps/router/src/languages/hu.json (1 hunks)
- apps/router/src/languages/it.json (1 hunks)
- apps/router/src/languages/ja.json (1 hunks)
- apps/router/src/languages/ko.json (1 hunks)
- apps/router/src/languages/pt.json (1 hunks)
- apps/router/src/languages/ru.json (1 hunks)
- apps/router/src/languages/zh.json (1 hunks)
- apps/router/src/react-app-env.d.ts (1 hunks)
- apps/router/tsconfig.json (1 hunks)
- mprocs-nix-guardian.yml (1 hunks)
- package.json (1 hunks)
- packages/types/src/federation.ts (3 hunks)
- packages/ui/src/Header.tsx (2 hunks)
- packages/ui/src/Wrapper.tsx (1 hunks)
- packages/ui/tsconfig.json (1 hunks)
- scripts/mprocs-nix-gateway.sh (1 hunks)
- scripts/mprocs-nix-guardian-manual.sh (1 hunks)
- scripts/mprocs-nix-guardian.sh (1 hunks)
- scripts/translate.js (1 hunks)
- turbo.json (2 hunks)
Files not reviewed due to no reviewable changes (29)
- apps/gateway-ui/public/index.html
- apps/gateway-ui/public/manifest.json
- apps/gateway-ui/src/ApiProvider.tsx
- apps/gateway-ui/src/App.tsx
- apps/gateway-ui/src/index.tsx
- apps/gateway-ui/tsconfig.json
- apps/guardian-ui/.eslintrc.js
- apps/guardian-ui/package.json
- apps/guardian-ui/public/config.json.example
- apps/guardian-ui/public/robots.txt
- apps/guardian-ui/src/App.tsx
- apps/guardian-ui/src/AppContext.tsx
- apps/guardian-ui/src/admin/AdminContext.tsx
- apps/guardian-ui/src/components/NotConfigured.tsx
- apps/guardian-ui/src/hooks/index.tsx
- apps/guardian-ui/src/index.tsx
- apps/guardian-ui/src/languages/ca.json
- apps/guardian-ui/src/languages/de.json
- apps/guardian-ui/src/languages/es.json
- apps/guardian-ui/src/languages/fr.json
- apps/guardian-ui/src/languages/hu.json
- apps/guardian-ui/src/languages/it.json
- apps/guardian-ui/src/languages/ja.json
- apps/guardian-ui/src/languages/ko.json
- apps/guardian-ui/src/languages/pt.json
- apps/guardian-ui/src/languages/ru.json
- apps/guardian-ui/src/languages/zh.json
- apps/guardian-ui/src/setup/SetupContext.tsx
- apps/guardian-ui/tsconfig.json
Files skipped from review due to trivial changes (13)
- apps/router/public/config.json
- apps/router/public/index.html
- apps/router/src/gateway-ui/components/federations/FederationsTable.tsx
- apps/router/src/gateway-ui/components/walletCard/WalletCard.tsx
- apps/router/src/languages/ca.json
- apps/router/src/languages/es.json
- apps/router/src/languages/fr.json
- apps/router/src/languages/hu.json
- apps/router/src/languages/ja.json
- apps/router/src/languages/ko.json
- mprocs-nix-guardian.yml
- package.json
- packages/types/src/federation.ts
Additional context used
Biome
apps/router/src/context/hooks.tsx
[error] 220-220: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (105)
apps/router/src/react-app-env.d.ts (1)
1-7
: LGTM!The TypeScript declaration file is correctly referencing the types for
react-scripts
and declaring the module for importing font files with the.ttf
extension.apps/router/tsconfig.json (1)
1-8
: LGTM!The TypeScript configuration file looks good. It extends the base configuration, specifies additional type roots, and includes/excludes the appropriate files and directories. I don't see any issues or problems with this configuration.
apps/router/src/guardian-ui/hooks/index.tsx (1)
3-14
: LGTM!The custom
useEllipsis
hook is implemented correctly. It manages the ellipsis state usinguseState
, sets up an interval to update the state usinguseEffect
, and returns the current ellipsis state. The interval is also correctly cleared in the cleanup function.scripts/mprocs-nix-guardian.sh (1)
20-20
: Verify alignment with PR objectives.The simplified
yarn dev
command looks good. However, please confirm whether this change is part of the larger refactoring effort to unify the Fedimint UI into a single application, as mentioned in the PR objectives.scripts/mprocs-nix-guardian-manual.sh (1)
27-27
: Verify if the new commandyarn dev
still starts the guardian UI as expected.The command to start the guardian UI has been changed from
yarn dev:guardian-ui
toyarn dev
. Please ensure that the new command still starts the guardian UI correctly and provides all the necessary functionalities.apps/router/src/context/guardian/AdminContext.tsx (1)
1-23
: LGTM, but consider providing a default value for theAdminContext
.The code segment is correctly implemented and follows the React context pattern. However, consider providing a default value for the
AdminContext
that matches the expected shape of the context value instead ofnull
.packages/ui/src/Wrapper.tsx (2)
2-2
: LGTM!The import statement is correct and necessary for the changes made in this file.
15-15
: LGTM!The changes improve the responsiveness of the
Wrapper
component by automatically adjusting its size based on the viewport size using theuseBreakpointValue
hook. This eliminates the need for the parent component to explicitly set thesize
prop.scripts/mprocs-nix-gateway.sh (1)
44-44
: LGTM!The change from
yarn dev:gateway-ui
toyarn dev
aligns with the PR objective of refactoring the Fedimint UI into a unified application.apps/router/src/guardian-ui/components/setup/screens/setupComplete/SetupComplete.tsx (1)
4-9
: LGTM!The changes in this file are focused on refactoring the component to align with the Guardian-specific context and action types. The component logic remains the same, with the only changes being the import statements and the specific context and action types used. The changes look good and do not introduce any new issues.
Also applies to: 17-17, 21-24
apps/router/src/gateway-ui/types.ts (1)
1-59
: LGTM!The types and enums in this file are well-defined and follow best practices. The
GatewayAppState
interface andGatewayAppAction
union type provide a clear structure for managing the state and actions in the gateway UI app. The code is clean and consistent.apps/router/package.json (3)
2-2
: LGTM!The renaming of the project from
@fedimint/gateway-ui
to@fedimint/router
is a valid change.
11-11
: LGTM!Enforcing stricter linting rules by adding the
--max-warnings 0
flag is a good practice to maintain code quality.
15-29
: LGTM!The updates to the existing dependencies and the addition of new dependencies are valid changes. They may bring bug fixes, performance improvements, and new features to the project.
The addition of
@codemirror
related dependencies suggests an enhancement in code editing capabilities, likely for JSON files. And the addition ofreact-router-dom
indicates that routing functionality is now part of the project, which is essential for single-page applications.turbo.json (3)
23-32
: LGTM!The addition of the
"router#build"
target with its specified outputs, dependencies, and environment variables looks good. It reflects a structured approach to managing dependencies during the build process.
37-41
: LGTM!The modification of the
"lint"
target to include outputs and dependencies on the build process looks good. It suggests a more integrated linting process that could potentially improve code quality checks.
74-81
: LGTM!The addition of the
"@fedimint/router#dev"
development target with its specified dependencies looks good. It enhances the modularity and maintainability of the codebase by clearly defining the relationships between components during development.apps/router/src/gateway-ui/components/walletModal/receive/ReceiveEcash.tsx (1)
9-9
: LGTM!The changes to use the
useGatewayApi
hook for accessing the API functionality are a good refactor. It enhances the component's integration with React's context system, promoting better state management and reactivity to context changes.Also applies to: 22-22, 28-28
apps/router/src/context/guardian/GuardianContext.tsx (1)
1-81
: LGTM!The code follows a standard and clear pattern for creating a React context with a reducer. The types and interfaces are well-defined, and the custom hooks are used appropriately for loading data and memoizing the API instance. There are no apparent issues or errors in the code.
apps/router/src/guardian-ui/components/dashboard/admin/InviteCode.tsx (2)
20-20
: LGTM!The change from
QRCode
toQRCodeSVG
is a good improvement for better rendering quality and scalability of the QR code.
Line range hint
75-82
: LGTM!The usage of the
QRCodeSVG
component to render the QR code with the invite code looks good. The size and styling of the QR code are appropriate for responsive scaling.apps/router/src/gateway-ui/Gateway.tsx (1)
19-68
: LGTM!The component structure and rendering logic are properly implemented. The conditional rendering based on
state.gatewayInfo
ensures that the main content is only rendered when the necessary data is available. The child components are composed in a clear and readable manner, with relevant props passed down from the parent component.apps/router/src/gateway-ui/components/walletModal/send/SendEcash.tsx (2)
8-8
: LGTM!The import statement for the custom hook
useGatewayApi
looks good.
Line range hint
24-43
: LGTM!The changes to replace the usage of
gateway
object withapi
object from the custom hookuseGatewayApi
are implemented correctly. ThehandleCreateEcash
function and the dependencies of theuseCallback
hook have been updated accordingly.apps/router/src/guardian-ui/Guardian.tsx (1)
17-80
: LGTM!The component logic implemented using the
useMemo
hook is correct. It handles different states such as error, authentication, setup, admin, and loading appropriately. Thecontent
variable is derived based on the state values and rendered accordingly.apps/router/src/guardian-ui/components/dashboard/danger/DownloadBackup.tsx (1)
16-16
: LGTM!The change from
useAdminContext
touseGuardianAdminApi
hook seems to be a refactor and does not introduce any new functionality. The rest of the component code remains unchanged.Also applies to: 21-21
apps/router/src/gateway-ui/components/walletModal/receive/ReceiveLightning.tsx (2)
8-8
: LGTM!The import statement for the custom hook
useGatewayApi
is correct and follows the proper syntax.
24-24
: LGTM!The usage of the
api
variable from the custom hookuseGatewayApi
is correct and maintains the existing functionality of creating a Bolt11 invoice. The component also correctly responds to updates in theapi
variable due to the adjusted dependency array.Also applies to: 36-43, 50-54
apps/router/src/gateway-ui/components/walletModal/receive/ReceiveOnchain.tsx (1)
8-8
: LGTM!The refactor of the API context access using the custom hook
useGatewayApi
improves the modularity and testability of the code. The type safety enhancement for thenewAddress
variable and the updated dependency array for theuseCallback
hook are also positive changes.Also applies to: 24-24, 33-35, 45-45
apps/router/src/guardian-ui/components/dashboard/admin/FederationInfoCard.tsx (2)
13-13
: LGTM!The import statement change looks good and is unlikely to introduce any issues.
27-27
: LGTM!The
api
variable change is consistent with the updated import statement and looks good.apps/router/src/gateway-ui/components/walletModal/send/SendOnchain.tsx (2)
16-16
: LGTM!Using a custom hook to access the API is a good practice for modularity and testability.
32-32
: LGTM!The usage of the
useGatewayApi
hook and theapi.submitPegOut
method call are correct. There are no issues with the logic of submitting a peg-out transaction.Also applies to: 70-74
apps/router/src/context/gateway/GatewayContext.tsx (3)
71-74
: Verify the usage of custom hooks.The component uses several custom hooks:
useSelectedServiceId
to get the selected service ID.useGatewayConfig
to get the gateway configuration.useLoadGateway
to load the gateway and check authentication status.Ensure that these hooks are correctly implemented and provide the expected values. Verify that the
useLoadGateway
hook correctly updates the authentication status and runs the initial auth check.
78-80
: Check the error handling for unauthenticated and error states.The component renders different states based on the authentication status and error state:
- It renders a loading state when running the initial auth check or when the state or gateway info is null.
- It renders an error message if there is a gateway error in the state.
Ensure that the loading and error states are correctly handled and provide a good user experience. Verify that the
ErrorMessage
component correctly displays the error message.
81-94
: Review the login component props and usage.The component renders a
Login
component when the user is not authenticated. It passes several props to theLogin
component:
checkAuth
prop with thegatewayApi.testPassword
method to check the password.setAuthenticated
prop with a dispatch action to update theneedsAuth
state.parseError
prop with a function to parse the error message.Ensure that the
Login
component is correctly implemented and handles the authentication flow properly. Verify that thecheckAuth
andsetAuthenticated
props work as expected and update the authentication state correctly.apps/router/src/guardian-ui/components/setup/RunDKG.tsx (4)
10-10
: LGTM!The import statement update aligns with the renaming of
ServerStatus
toGuardianServerStatus
in the@fedimint/types
module.
14-18
: LGTM!The import statement updates align with the changes to the context hooks and the introduction of the
useGuardianSetupApi
hook.
26-29
: LGTM!The changes to the
api
variable assignment and the usage ofuseGuardianSetupContext
align with the updates to the context hooks and the introduction of theuseGuardianSetupApi
hook.
Line range hint
48-83
: LGTM!The updates to the
ServerStatus
references, now usingGuardianServerStatus
, align with the renaming changes. The logic flow remains intact and consistent.apps/router/src/guardian-ui/components/setup/screens/roleSelector/RoleSelector.tsx (2)
22-22
: LGTM!
32-32
: LGTM!apps/router/src/gateway-ui/components/ConnectFederationModal.tsx (5)
16-16
: LGTM!The
GatewayInfo
type is correctly imported and used in the component.
18-22
: LGTM!The hooks from the context provider are correctly imported and used in the component. This change improves the modularity and maintainability of the code.
36-38
: LGTM!The
api
,dispatch
, andgatewayInfo
are correctly obtained using the respective hooks. This change improves the state management approach.
40-40
: LGTM!The
errorMsg
state variable is correctly initialized and used in the component.
53-62
: LGTM!The
handleConnectFederation
function correctly uses theapi
object to connect to the federation and dispatches an action to update the state upon a successful connection. This change streamlines the component's logic and improves the clarity of data flow.apps/router/src/guardian-ui/components/dashboard/gateways/GatewaysCard.tsx (2)
21-21
: LGTM!The import statement change looks good. It seems to be part of a refactor to improve the modularity or clarity of the codebase.
32-32
: LGTM!The API retrieval change is consistent with the import statement change and looks good.
apps/router/src/guardian-ui/components/dashboard/danger/GuardianAuthenticationCode.tsx (3)
15-15
: LGTM!The change from
QRCode
toQRCodeSVG
is a minor modification that doesn't affect the functionality of the component. Theqrcode.react
library is a reliable choice for generating QR codes in SVG format.
92-92
: LGTM!The adjustments in the translation keys are minor and don't introduce any issues. The removal of trailing commas ensures consistency in the translation key usage.
Also applies to: 116-116, 138-138
127-127
: LGTM!The usage of
QRCodeSVG
component is correct, and the provided props are appropriate for generating the QR code. Thesize
prop is set to a constant valueQR_CODE_SIZE
, and thestyle
prop is used to ensure that the QR code is responsive and has a maximum width equal toQR_CODE_SIZE
.apps/router/src/gateway-ui/components/walletModal/index.tsx (2)
21-21
: LGTM!The change from
QRCode
toQRCodeSVG
is a good improvement. Using SVG-based rendering for the QR code will enhance its scalability and visual quality without affecting the existing functionality.
135-135
: LGTM!The usage of
QRCodeSVG
component here is consistent with the import statement change. The QR code size remains the same, so this change looks good.apps/router/src/guardian-ui/types.tsx (1)
Line range hint
1-140
: No issues found. The changes look good!The refactoring improves naming consistency and clarity by prefixing relevant types and enums with "Guardian". This makes the code more readable and maintainable.
apps/router/src/gateway-ui/components/federations/ConnectFederation.tsx (1)
21-21
: LGTM!The refactor to use the
useGatewayApi
custom hook for API interactions instead of directly accessing theApiContext
improves code maintainability and readability. The changes align with the common pattern of using hooks for API interactions in React applications and promote modularity.Also applies to: 87-87, 100-111
apps/router/src/guardian-ui/admin/FederationAdmin.tsx (1)
17-17
: LGTM!The change to use
useGuardianAdminApi
instead ofuseAdminContext
looks good. It seems to be a refactor to use a more specific API context for guardian administration.Also applies to: 27-27
apps/router/src/guardian-ui/components/dashboard/tabs/meta/ViewConsensusMeta.tsx (2)
7-7
: LGTM!The change in the import statement for accessing the API context is approved.
32-32
: LGTM!The change in accessing the API context using the
useGuardianAdminApi
hook is approved.apps/router/src/context/guardian/SetupContext.tsx (4)
25-64
: LGTM!The
makeInitialState
function is correctly implemented and there are no issues with the logic.
68-93
: LGTM!The
reducer
function is correctly implemented and there are no issues with the logic.
95-115
: LGTM!The
SetupContext
andSetupContextValue
are correctly defined and there are no issues with the types.
122-158
: LGTM!The
SetupContextProvider
component is correctly implemented and there are no issues with the logic.apps/router/src/guardian-ui/components/setup/screens/connectGuardians/ConnectGuardians.tsx (3)
18-18
: LGTM!The import statement for
GuardianServerStatus
is correct and necessary.
23-26
: LGTM!The import statements for
useConsensusPolling
anduseGuardianSetupContext
hooks are correct and necessary.
46-48
: LGTM!The filtering logic for checking if all peers are ready for config generation is correct and uses the appropriate enum value.
apps/router/src/guardian-ui/components/dashboard/danger/SignApiAnnouncement.tsx (2)
24-24
: LGTM!The import statement change from
useAdminContext
touseGuardianAdminApi
is consistent with the AI-generated summary and may reflect a broader architectural change in the application.
37-37
: LGTM!The change in accessing the API context using
useGuardianAdminApi()
is consistent with the updated import statement and indicates the usage of a more specialized or updated context for managing API interactions related to the Guardian admin functionalities. The logic of the component remains intact.scripts/translate.js (2)
19-19
: Verify the impact of hardcoding the source path.The change eliminates the flexibility to select source paths based on the target file. Ensure that hardcoding the path to
apps/router/src/languages
does not break any existing functionality or use case of the script.
20-20
: Clarify the purpose and impact of settingtargetKey
tofalse
.Setting
targetKey
tofalse
instead of deriving it fromprocess.argv[3]
may indicate a change in how keys are handled in the translation process. Please provide more context on the purpose of this change and verify that it does not introduce any unintended consequences.apps/router/src/guardian-ui/components/dashboard/tabs/meta/MetaManager.tsx (2)
23-23
: LGTM!The import change looks good. It indicates a shift to using the
useGuardianAdminApi
hook for accessing the API context.
57-57
: LGTM!The change in assigning the
api
variable directly fromuseGuardianAdminApi()
looks good. It aligns with the updated import statement and suggests a shift in the API context management.apps/router/src/guardian-ui/setup/FederationSetup.tsx (4)
21-26
: LGTM!The import statements are correct and necessary for the component.
39-39
: LGTM!The code change is correct.
43-43
: LGTM!The code change is correct.
153-153
: LGTM!The code change is correct.
packages/ui/src/Header.tsx (1)
Line range hint
22-56
: LGTM!The changes to wrap the header content in an anchor tag with a link to the homepage are straightforward and improve the user experience by making the header clickable.
apps/router/src/guardian-ui/components/setup/screens/setConfiguration/SetConfiguration.tsx (3)
29-32
: LGTM!The changes to import
useGuardianSetupApi
anduseGuardianSetupContext
hooks align with the refactoring objective to use more specialized context and API hooks for the guardian setup process.
42-42
: LGTM!The change to use the
useGuardianSetupApi
hook to get theapi
object aligns with the refactoring objective to use more specialized API hooks for the guardian setup process.
52-52
: LGTM!The change to use the
useGuardianSetupContext
hook to get the state andsubmitConfiguration
function aligns with the refactoring objective to use more specialized context for the guardian setup process.apps/router/src/guardian-ui/GuardianApi.ts (7)
11-11
: LGTM!The change from
ServerStatus
toGuardianServerStatus
is consistent with the broader reorganization of status management within the codebase.
21-21
: LGTM!Making the
fm_config_api
property mandatory in theGuardianConfig
type ensures that the configuration is always available at the time of instantiation, which simplifies the control flow and enhances reliability.
28-28
: LGTM!Adding a constructor that accepts a
GuardianConfig
object enforces the configuration requirements at instantiation, which eliminates the need for asynchronous fetching of the configuration from a JSON file.
32-32
: LGTM!Removing the
websocketUrl
parameter from theconnect
method is consistent with the constructor modification, which provides thefm_config_api
directly from the instance's configuration.
43-43
: LGTM!Using the
fm_config_api
directly from the instance's configuration in theconnect
method is consistent with the constructor modification and simplifies the connection logic.
185-188
: LGTM!The change from
ServerStatus
toGuardianServerStatus
is consistent with the broader reorganization of status management within the codebase.
301-301
: LGTM!Adding a check to ensure that the
fm_config_api
property exists in theguardianConfig
before making the API call enhances reliability and prevents potential errors.apps/router/src/guardian-ui/components/dashboard/tabs/meta/ProposedMetas.tsx (1)
32-32
: LGTM!The change in the API access mechanism from
useAdminContext
touseGuardianAdminApi
is a straightforward refactor that doesn't introduce any issues. The component's functionality remains unaffected by this change.Also applies to: 62-62
apps/router/src/guardian-ui/components/setup/screens/verifyGuardians/VerifyGuardians.tsx (3)
57-61
: Ensure proper error handling and state updates.The component uses the
useGuardianSetupApi
hook to access the API and theuseGuardianSetupContext
hook to access the setup context. Ensure that any errors thrown by the API calls are properly handled and the state is updated accordingly. Consider adding error boundaries or error handling components to provide a better user experience.
77-81
: Verify the logic for checking peer status.The component checks if all peers have a status of
GuardianServerStatus.VerifiedConfigs
to determine if the configurations are verified. Ensure that this logic is correct and matches the expected behavior of the guardian setup flow.
113-115
: Verify the logic for prefilling hashes.The component prefills the hashes of other peers if the current peer's status is
GuardianServerStatus.VerifiedConfigs
. Ensure that this logic is correct and does not introduce any security vulnerabilities. Consider adding validation or sanitization for the prefilled hashes.apps/router/src/gateway-ui/GatewayApi.ts (2)
23-23
: LGTM!Making
baseUrl
a required property is a good practice to ensure it always has a valid value.
25-27
: LGTM!The constructor ensures that
baseUrl
is initialized with a valid value when an instance ofGatewayApi
is created.apps/router/src/languages/zh.json (1)
1-486
: Translations look good overall, but recommend review by native speaker.The Chinese translations in this file look reasonable based on a spot check. However, recommend having a native Chinese speaker review the full file to:
- Ensure the translations are accurate and read naturally
- Check that all the needed translations are present and match the original English version
apps/router/src/languages/en.json (4)
20-29
: LGTM!
355-362
: LGTM!
363-373
: LGTM!
374-484
: LGTM!apps/router/src/languages/pt.json (2)
1-486
: JSON structure and syntax look good!The file contains a well-formed JSON object with correct syntax. All strings are properly enclosed in double quotes.
1-486
: Placeholders are used correctly.The JSON strings contain placeholders like
{{key}}
to insert dynamic values. They are used correctly to compose messages.apps/router/src/languages/ru.json (1)
1-486
: LGTM!apps/router/src/languages/it.json (1)
1-486
: LGTM!The Italian translations in this file look good. The JSON structure is valid and the translations seem appropriate.
apps/router/src/languages/de.json (1)
1-486
: Localization file looks good, but translations need verification.The German localization JSON file follows the correct structure and syntax. However, the accuracy of the translations should be verified separately by a native German speaker before releasing to users.
apps/router/src/gateway-ui/components/HeaderWithUnitSelector.tsx (1)
Line range hint
8-36
: LGTM!The refactor to use context for state management instead of props is a good improvement. It enhances the component's encapsulation and aligns it with a more centralized state management strategy. The logic and implementation look correct.
apps/router/src/index.tsx (1)
1-61
: LGTM!The code structure and routing setup look good. The use of context providers and conditional rendering of routes based on the selected service is a clean and modular approach. I don't see any major issues or problems with the code.
apps/router/src/home/HomePage.tsx
Outdated
<Modal isOpen={isOpen} onClose={onClose}> | ||
<ModalOverlay /> | ||
<ModalContent> | ||
<ModalHeader>{t('home.addGuardian', 'Add Guardian')}</ModalHeader> | ||
<ModalCloseButton /> | ||
<ModalBody pb={6}> | ||
<FormControl> | ||
<FormLabel>{t('notConfigured.urlLabel')}</FormLabel> | ||
<Input | ||
placeholder='wss://fedimintd.my-awesome-domain.com:6000' | ||
value={configUrl} | ||
onChange={(e) => setConfigUrl(e.target.value)} | ||
/> | ||
</FormControl> | ||
<Button mt={4} colorScheme='blue'> | ||
{t('common.submit')} | ||
</Button> | ||
</ModalBody> | ||
</ModalContent> | ||
</Modal> |
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.
Add a form submission handler to the modal.
The modal for adding a Guardian or Gateway is missing a form submission handler. When the user clicks the submit button, the entered configuration URL should be validated, and the list of guardians or gateways should be updated accordingly.
apps/router/src/home/HomePage.tsx
Outdated
{Object.keys(guardians).length + Object.keys(gateways).length === 0 ? ( | ||
<> | ||
<Button | ||
onClick={onOpen} | ||
colorScheme='green' | ||
mb={4} | ||
width='100%' | ||
maxWidth='400px' | ||
> | ||
{t('home.connectGuardian', 'Connect a Guardian')} | ||
</Button> | ||
<Button | ||
onClick={onOpen} | ||
colorScheme='purple' | ||
mb={8} | ||
width='100%' | ||
maxWidth='400px' | ||
> | ||
{t('home.connectGateway', 'Connect a Gateway')} | ||
</Button> | ||
</> | ||
) : ( |
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.
Clarify the distinction between adding a Guardian and a Gateway.
The current implementation doesn't clearly distinguish between adding a Guardian and a Gateway. The modal opened by both the "Connect a Guardian" and "Connect a Gateway" buttons only mentions adding a Guardian in its header.
Consider updating the modal to clearly indicate whether the user is adding a Guardian or a Gateway, and handle the form submission accordingly. This could be achieved by passing a prop to the modal to specify the type of service being added, and updating the modal header and form submission handler based on that prop.
Also applies to: 104-123
"from-federation": "Da Federação", | ||
"sent": "Enviado", | ||
"sent-amount": "Enviado {{amount}}", | ||
"to-address": "Para Endereço", | ||
"txid": "ID da Transação", | ||
"claimed-note": "Nota Reivindicada", | ||
"claimed-amount": "Valor Reivindicado" | ||
}, | ||
"wallet-modal": { | ||
"title": "Ações da Carteira", | ||
"receive": { | ||
"ecash-instructions": "Cole seu bilhete de ecash aqui", | ||
"paste-ecash-placeholder": "Cole aqui a nota de ecash", | ||
"redeem": "Resgatar", | ||
"paste-invoice": "Cole aqui a fatura", | ||
"enter-amount": "Insira a quantidade em {{unit}}", | ||
"lightning-instructions": "Insira um valor em sats para criar uma fatura Lightning para", | ||
"create-peg-in-address": "Criar Endereço Peg-in", | ||
"create-lightning-invoice": "Crie uma fatura Lightning", | ||
"paste-ecash-button": "Cole nota de ecash", | ||
"peg-in-instructions": "Envie Bitcoin para este endereço para vincular ao seu saldo Ecash da {{federationName}}", | ||
"ecash-claimed-success": "Ecash reivindicado com sucesso!", | ||
"address-error": "Erro ao criar endereço: {{error}}" | ||
}, | ||
"send": { | ||
"submit": "Enviar", | ||
"to-onchain-address": "Para Endereço Onchain", | ||
"address-placeholder": "bc1p...", | ||
"peg-out-to-onchain": "Transferir Ecash para Onchain", | ||
"peg-out-success": "Peg Out TX Enviado!", | ||
"create-ecash": "Criar Nota Ecash", | ||
"ecash-created": "Retirou uma nota de ecash de {{amount}} do seu saldo {{federationName}}. Envie-a para o destinatário para reivindicar.", | ||
"ecash-error": "Erro ao criar nota ecash: {{error}}" | ||
} | ||
}, | ||
"federation-card": { | ||
"table-title": "Federações", | ||
"id": "ID", | ||
"name": "Nome", | ||
"balance": "Equilíbrio", | ||
"actions": "Ações", | ||
"receive": "Receber", | ||
"send": "Enviar", | ||
"default-federation-name": "Federação:", | ||
"view-link-on": "Visualizar em {{host}}", | ||
"leave-fed-error": "Não pode sair da federação com sats no seu saldo. Por favor, retire seus sats primeiro.", | ||
"leave-fed-modal-title": "Deixe a Federação", | ||
"leave-fed-modal-text": "Tem certeza de que deseja se desconectar de", | ||
"view-config": "Visualizar Configuração", | ||
"config-for": "Configuração para {{federationId}}" | ||
}, | ||
"header": { | ||
"active": "Ativo", | ||
"all": "Todos", | ||
"archived": "Arquivado", | ||
"ascending": "Ascendendo", | ||
"connect": "Conectar Federação", | ||
"date-created": "Data de Criação", | ||
"descending": "Descendente", | ||
"filter": "Filtro", | ||
"sort": "Ordenar", | ||
"title": "Painel de Controle do Gateway de Relâmpago" | ||
}, | ||
"info-card": { | ||
"card-header": "Informações do Nó de Relâmpago", | ||
"node-id": "ID do Nó", | ||
"alias": "Apelido", | ||
"mode": "Modo", | ||
"pubkey": "Chave pública", | ||
"network": "Rede", | ||
"block-height": "Altura do Bloco", | ||
"synced-to-chain": "Sincronizado com a Corrente" | ||
}, | ||
"withdraw-card": { | ||
"address-label": "O seu endereço:", | ||
"address-placeholder": "Insira o seu endereço btc", | ||
"amount-label": "Quantidade (sats):", | ||
"amount-placeholder": "Insira a quantidade em sats", | ||
"card-header": "Retirar Bitcoin", | ||
"close": "Fechar", | ||
"confirm-withdraw": "Confirmar Retirada", | ||
"error": "Erro", | ||
"error-address": "O valor ou endereço não podem estar vazios", | ||
"error-amount": "O valor não pode estar vazio ou ser igual a zero", | ||
"error-request": "Falha ao solicitar a retirada", | ||
"request-from": "Solicitada Retirada de", | ||
"requested-withdrawal": "Solicitação de Saque", | ||
"to": "para", | ||
"total_bitcoin": "Total de Bitcoin:", | ||
"transaction-sent": "Transação Enviada!", | ||
"view": "Visualizar", | ||
"view-it-on": "Veja isso em", | ||
"withdraw": "Retirar", | ||
"withdraw-all": "Retirar tudo", | ||
"withdrawal-created": "Saque criado.", | ||
"withdrawal-created-description": "Por favor, verifique seu histórico de transações", | ||
"withdrawal-history": "Histórico de Saques", | ||
"your-transaction": "O ID da sua transação:" | ||
} | ||
} |
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.
Have a native Portuguese speaker review the translations for accuracy and completeness.
The Portuguese translations in this file need to be carefully reviewed by a native speaker of the language to ensure they accurately convey the meaning of the source strings. Please have a linguistic expert review the strings in this file against the original English version.
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 (1)
README.md (1)
14-14
: Refactoring the UI into a single application is a great improvement!The unified client-side app running on both browser and desktop will significantly enhance the user experience.
Consider using a hyphen for the compound adjective:
-We've refactored the Fedimint UI into a single client side app that can run on the browser or on desktop using Electron. +We've refactored the Fedimint UI into a single client-side app that can run on the browser or on desktop using Electron.Tools
LanguageTool
[uncategorized] ~14-~14: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...efactored the Fedimint UI into a single client side app that can run on the browser or on d...(EN_COMPOUND_ADJECTIVE_INTERNAL)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- README.md (1 hunks)
- apps/router/src/context/AppContext.tsx (1 hunks)
- apps/router/src/context/gateway/GatewayContext.tsx (1 hunks)
- apps/router/src/context/guardian/GuardianContext.tsx (1 hunks)
- apps/router/src/context/hooks.tsx (1 hunks)
- apps/router/src/gateway-ui/Gateway.tsx (1 hunks)
- apps/router/src/gateway-ui/components/lightning/LightningCard.tsx (1 hunks)
- apps/router/src/guardian-ui/Guardian.tsx (1 hunks)
- apps/router/src/guardian-ui/GuardianApi.ts (5 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceTable.tsx (1 hunks)
- apps/router/src/guardian-ui/utils/env.ts (1 hunks)
- apps/router/src/home/ConnectServiceModal.tsx (1 hunks)
- apps/router/src/home/HomePage.tsx (1 hunks)
- apps/router/src/index.tsx (1 hunks)
- apps/router/src/setupProxy.js (1 hunks)
- packages/utils/src/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceTable.tsx
Files skipped from review as they are similar to previous changes (6)
- apps/router/src/context/AppContext.tsx
- apps/router/src/context/guardian/GuardianContext.tsx
- apps/router/src/gateway-ui/Gateway.tsx
- apps/router/src/guardian-ui/Guardian.tsx
- apps/router/src/home/HomePage.tsx
- apps/router/src/index.tsx
Additional context used
LanguageTool
README.md
[uncategorized] ~14-~14: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...efactored the Fedimint UI into a single client side app that can run on the browser or on d...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...r on desktop using Electron. -router
: This creates a homescreen that shows al...(UNLIKELY_OPENING_PUNCTUATION)
Biome
apps/router/src/context/hooks.tsx
[error] 224-224: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (27)
apps/router/src/setupProxy.js (1)
1-13
: LGTM!The middleware function is correctly checking for the presence of environment variables and responding with the appropriate JSON configuration. The implementation looks good.
apps/router/src/guardian-ui/utils/env.ts (1)
9-9
: LGTM!The change in the condition and error message is consistent with the new expected structure of the configuration object.
packages/utils/src/index.tsx (1)
22-30
: LGTM!The
sha256Hash
function correctly computes the SHA-256 hash of the input string using the Web Crypto API. The implementation looks good.apps/router/src/context/gateway/GatewayContext.tsx (1)
1-83
: LGTM!The
GatewayContext
andGatewayContextProvider
implementation follows best practices and is well-structured. The code is easy to understand and uses appropriate types, interfaces, naming conventions, error handling, state management, memoization, hooks, routing, comments, and formatting.apps/router/src/home/ConnectServiceModal.tsx (1)
1-104
: LGTM!The
ConnectServiceModal
component is well-structured and follows best practices for React components. The use of React hooks for state management, the validation of the URL format, and the prevention of duplicate entries are all important features. The dispatching of actions to add the guardian or gateway configuration to the application context is a good way to manage the application state. The use of Chakra UI for the user interface elements and the support for internationalization through theuseTranslation
hook are also good practices.apps/router/src/guardian-ui/GuardianApi.ts (6)
11-11
: LGTM!
21-21
: LGTM!
28-28
: LGTM!
32-32
: LGTM!
43-43
: LGTM!
188-188
: LGTM!README.md (1)
16-16
: The newrouter
application is a valuable addition.Providing a homescreen that shows all connected services will greatly improve the user's ability to manage and navigate between different Fedimint services.
Tools
LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...r on desktop using Electron. -router
: This creates a homescreen that shows al...(UNLIKELY_OPENING_PUNCTUATION)
apps/router/src/context/hooks.tsx (15)
43-45
: LGTM!The function correctly uses the
useContext
hook to access theAppContext
.
47-50
: LGTM!The function correctly uses the
useAppContext
hook to access theguardians
object and maps over it to return an array ofconfig
properties.
52-54
: LGTM!The function correctly uses the
useAppContext
hook to access theguardians
object and returns the length of its keys.
56-58
: LGTM!The function correctly uses the
useAppContext
hook to access thegateways
object and returns the length of its keys.
60-65
: LGTM!The function correctly uses the
useAppContext
hook to access theguardians
object and returns theconfig
property of the guardian with the givenid
. It also throws an error if the guardian with the givenid
does not exist.
67-72
: LGTM!The function correctly uses the
useAppContext
hook to access thegateways
object and returns theconfig
property of the gateway with the givenid
. It also throws an error if the gateway with the givenid
does not exist.
74-81
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns itsdispatch
property. It also throws an error if theGuardianContext
does not exist.
83-136
: LGTM!The function correctly uses the
useGuardianApi
,useGuardianState
, anduseGuardianDispatch
hooks to load the guardian data and update the state accordingly. It handles the different server statuses and sets the appropriate state values. It also handles errors by setting the error message in the state.
138-145
: LGTM!The function correctly uses the
useContext
hook to access theGuardianContext
and returns its value. It also throws an error if theGuardianContext
does not exist.
147-150
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns itsapi
property.
152-155
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns itsstate
property.
157-161
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns thestatus
property of its state. It also returns theLoading
status if theGuardianContext
does not exist.
163-166
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns itsapi
property.
168-175
: LGTM!The function correctly uses the
useContext
hook to access theSetupContext
and returns its value. It also throws an error if theSetupContext
does not exist.
177-180
: LGTM!The function correctly uses the
useGuardianContext
hook to access theGuardianContext
and returns itsapi
property.
if (!this.guardianConfig?.baseUrl) { | ||
throw new Error('fm_config_api not found in config.json'); | ||
} | ||
const websocket = await this.connect(this.guardianConfig?.fm_config_api); | ||
const websocket = await this.connect(); |
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.
Remove unnecessary check.
The check for this.guardianConfig?.baseUrl
is unnecessary because the GuardianConfig
type now requires a baseUrl
property, and the constructor ensures that the configuration is always provided upon instantiation.
Apply this diff to remove the unnecessary check:
-if (!this.guardianConfig?.baseUrl) {
- throw new Error('fm_config_api not found in config.json');
-}
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.
if (!this.guardianConfig?.baseUrl) { | |
throw new Error('fm_config_api not found in config.json'); | |
} | |
const websocket = await this.connect(this.guardianConfig?.fm_config_api); | |
const websocket = await this.connect(); | |
const websocket = await this.connect(); |
if (!this.guardianConfig?.baseUrl) { | ||
throw new Error('fm_config_api not found in config.json'); | ||
} | ||
await this.connect(this.guardianConfig?.fm_config_api); | ||
await this.connect(); |
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.
Remove unnecessary check.
The check for this.guardianConfig?.baseUrl
is unnecessary because the GuardianConfig
type now requires a baseUrl
property, and the constructor ensures that the configuration is always provided upon instantiation.
Apply this diff to remove the unnecessary check:
-if (!this.guardianConfig?.baseUrl) {
- throw new Error('fm_config_api not found in config.json');
-}
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.
if (!this.guardianConfig?.baseUrl) { | |
throw new Error('fm_config_api not found in config.json'); | |
} | |
await this.connect(this.guardianConfig?.fm_config_api); | |
await this.connect(); | |
await this.connect(); |
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.
Tested locally, works for me!
This refactors the entire fedimint UI into a single app where you can add connections, gateways or guardians, and manage multiples of those fedimint services from a single UI. Paired the whole thing with @alexlwn123 , who is the bomb.
The goal here is for us to not have to "deploy" the UIs anymore, people instead install it on whatever device/format they want and only have to concern themselves with deploying the backend services.
people will be able to install the UI on desktop using electron, mobile using capacitor, or even run a PWA off the github pages(less secure but easier), and client side connect it to whatever fedimintds or gatewayds they've deployed.
Summary by CodeRabbit
New Features
HomePage
component for users to view and manage connected services, including adding new services.Bug Fixes
Refactor
GuardianApi
class for better configuration handling and connection logic.Chores