-
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
feat: switch everything to tabs #560
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant structural changes to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🔇 Additional comments (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: 7
🧹 Outside diff range and nitpick comments (3)
apps/router/src/languages/zh.json (1)
Line range hint
1-1
: Consider revising the translation for "claimed-note".The current translation "声称的笔记" (shēngchēng de bǐjì) might be misleading. In the context of cryptocurrency, "note" typically refers to a unit of digital currency rather than a written note. Consider using a term that better conveys the concept of a redeemed or claimed unit of digital currency.
apps/router/src/languages/ko.json (1)
Line range hint
1-1000
: Overall translation quality is good, but some improvements could be made.The Korean translations in this file are generally well-structured and consistent. However, there are a few areas where additional context or clarification could enhance user understanding:
- Technical terms: Consider adding explanations for technical terms that are left in English, such as "DKG" or "API announcements".
- Contextual translations: Some translations might benefit from more context-specific wording, especially in the setup and wallet sections.
These improvements could enhance the overall user experience for Korean-speaking users.
apps/router/src/languages/it.json (1)
Line range hint
270-270
: Redundant translation detected.On line 270, "actions" is translated as "Aktionen", which is inconsistent with the correct translation "Azione" used on line 11. This should be corrected to maintain consistency throughout the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (24)
- apps/router/src/guardian-ui/admin/FederationAdmin.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceCard.tsx (0 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceTable.tsx (0 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/FederationInfoCard.tsx (0 hunks)
- apps/router/src/guardian-ui/components/dashboard/admin/InviteCode.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/FederationTabsCard.tsx (6 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/balanceSheet/BalanceSheet.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/balanceSheet/BalanceTable.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/info/FederationInfo.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/status/FederationStatus.tsx (1 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/status/GatewaysStatus.tsx (2 hunks)
- apps/router/src/guardian-ui/components/dashboard/tabs/status/GuardiansStatus.tsx (3 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)
💤 Files with no reviewable changes (3)
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceCard.tsx
- apps/router/src/guardian-ui/components/dashboard/admin/BalanceTable.tsx
- apps/router/src/guardian-ui/components/dashboard/admin/FederationInfoCard.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/router/src/languages/hu.json
🧰 Additional context used
🔇 Additional comments (23)
apps/router/src/guardian-ui/components/dashboard/tabs/status/FederationStatus.tsx (3)
1-9
: Imports look good.
11-15
: Props interface is well-defined.
17-32
: FederationStatus component implementation is correct.apps/router/src/guardian-ui/admin/FederationAdmin.tsx (2)
2-11
: Import changes align with new tabbed layout.The removal of several component imports and the addition of
FederationTabsCard
are consistent with the PR's objective of implementing a tabbed interface.
Line range hint
79-93
: Component structure changes implement tabbed layout.The simplified structure with
InviteCode
,FederationTabsCard
, andDangerZone
as main components implements the desired tabbed layout. New props enhance information sharing between components.apps/router/src/guardian-ui/components/dashboard/tabs/status/GatewaysStatus.tsx (5)
16-18
: Import paths updated correctly.
22-26
: Component and props interface names updated consistently.
107-110
: Component structure simplified using Flex.
111-126
: Alert rendering adapted to new component structure.
127-130
: Table rendering simplified within new component structure.apps/router/src/languages/ja.json (1)
Line range hint
1-1037
: LGTM!The changes in this file are minimal and don't introduce any issues. The renaming of "fed-info" to "info" in the "federation-dashboard" section is a simple change that doesn't affect functionality.
apps/router/src/languages/en.json (4)
53-53
: Verify if guardian authentication is still needed.The removal of "authenticate as a guardian" might indicate a change in functionality. Ensure this aligns with any updates to the authentication process.
Line range hint
55-68
: LGTM. Verify UI updates for new labels.The restructuring and new labels improve the information display. Ensure the UI has been updated to accommodate these changes, especially for the new version and consensus block height information.
69-71
: Clarify implementation status of the new 'status' object.The new
status
object seems to be a placeholder. Confirm if this is intentional or if additional properties are planned for the status display.
Line range hint
53-71
: LGTM. Verify consistency with new tabbed layout.The changes align well with the PR objectives of switching to a tabbed layout and consolidating UI elements. Ensure all new and modified labels are correctly integrated into the new UI design.
apps/router/src/languages/pt.json (1)
55-55
: Key rename looks good.The change from "fed-info" to "info" is consistent and doesn't affect the underlying structure.
apps/router/src/languages/ru.json (1)
Line range hint
55-67
: Key renamed from "fed-info" to "info".The change is consistent with the summary and doesn't affect the translations.
apps/router/src/languages/es.json (1)
Line range hint
55-67
: Update code references to the renamed key.The key "fed-info" has been renamed to "info". Ensure all code referencing this key is updated accordingly.
✅ Verification successful
No issues found with key renaming. All references updated accordingly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of the old key name "fed-info" in JavaScript and TypeScript files rg '"fed-info"' -t js -t tsLength of output: 29
apps/router/src/languages/fr.json (1)
55-55
: Change approved.The renaming from "fed-info" to "info" is consistent with the AI summary and doesn't introduce any structural changes to the translations.
apps/router/src/languages/ca.json (1)
Line range hint
55-67
: Verify key renaming across the application.The key "fed-info" has been renamed to "info" in the "federation-dashboard" section. Ensure this change is reflected in the application code and other language files to maintain consistency.
✅ Verification successful
Adjust the search commands to correctly target TypeScript and JavaScript files.
Key renaming is consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for occurrences of "fed-info" in other language files and application code rg '"fed-info"' --type json rg 'fed-info' --type typescript --type javascriptLength of output: 114
Script:
#!/bin/bash # Check for occurrences of "fed-info" in other language files and application code rg '"fed-info"' --type json rg 'fed-info' -g "*.ts" -g "*.js"Length of output: 63
apps/router/src/languages/de.json (1)
Line range hint
55-67
: LGTM: Key renamed and label updated.The renaming of "fed-info" to "info" and the update of the label look good. The changes are consistent and maintain the overall structure of the translations.
apps/router/src/guardian-ui/components/dashboard/tabs/status/GuardiansStatus.tsx (1)
Line range hint
1-110
: LGTM!The changes align with the PR objectives, improving the component's structure and naming conventions while maintaining its core functionality.
apps/router/src/guardian-ui/components/dashboard/tabs/balanceSheet/BalanceTable.tsx (1)
1-143
: Code meets the requirements and functions correctly.No issues were found in the implementation.
useEffect(() => { | ||
const fetchBalance = () => { | ||
api.audit().then(setAuditSummary).catch(console.error); | ||
}; | ||
fetchBalance(); | ||
const interval = setInterval(fetchBalance, 5000); | ||
return () => clearInterval(interval); | ||
}, [api]); |
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.
Reconsider polling interval
The 5-second interval for fetching balance data might be too frequent. Consider increasing the interval to reduce server load and improve performance.
apps/router/src/guardian-ui/components/dashboard/tabs/balanceSheet/BalanceSheet.tsx
Show resolved
Hide resolved
apps/router/src/guardian-ui/components/dashboard/tabs/info/FederationInfo.tsx
Show resolved
Hide resolved
apps/router/src/guardian-ui/components/dashboard/tabs/info/FederationInfo.tsx
Show resolved
Hide resolved
apps/router/src/guardian-ui/components/dashboard/tabs/FederationTabsCard.tsx
Show resolved
Hide resolved
signedApiAnnouncements={signedApiAnnouncements} | ||
/> | ||
<GatewaysCard config={config} /> | ||
<InviteCode inviteCode={inviteCode} /> |
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.
I was expecting all files to be reformatted to use tabs instead of spaces. :D
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.
I can do that separately, that's a linter update
fde3d54
to
fdcdf40
Compare
switches the whole UI to just using tabs, keeps everything on one page so no more scrolling:
Summary by CodeRabbit
Summary by CodeRabbit
BalanceSheet
,BalanceTable
,FederationInfo
, andFederationStatus
.FederationTabsCard
with additional functionality and improved layout.Gateway
component for better content organization.FederationAdmin
component.FederationsTable
layout for improved flexibility.GatewaysCard
toGatewaysStatus
andGuardiansCard
toGuardiansStatus
.BalanceCard
,BalanceTable
, andFederationInfoCard
.