Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughConsolidates multiple settings pages/modals into a single client-side Changes
Sequence DiagramsequenceDiagram
actor User
participant Navbar
participant ModalSystem
participant MonarchSettingsModal
participant SettingsSidebar
participant SettingsContent
participant PanelsDetails
User->>Navbar: Click "Settings"
Navbar->>ModalSystem: openModal('monarchSettings', { initialCategory })
ModalSystem->>MonarchSettingsModal: open(isOpen=true, initialCategory)
MonarchSettingsModal->>SettingsSidebar: render(selectedCategory)
MonarchSettingsModal->>SettingsContent: render(category, detailView=null)
SettingsContent->>PanelsDetails: render Panel for category
User->>SettingsSidebar: Select "Preferences"
SettingsSidebar->>MonarchSettingsModal: onSelectCategory('preferences')
MonarchSettingsModal->>SettingsContent: update category
SettingsContent->>PanelsDetails: render PreferencesPanel
User->>PanelsDetails: Click "Manage Trusted Vaults"
PanelsDetails->>MonarchSettingsModal: onNavigateToDetail('trusted-vaults')
MonarchSettingsModal->>SettingsContent: set detailView, slideDirection='forward'
SettingsContent->>PanelsDetails: render TrustedVaultsDetail (animated)
User->>SettingsContent: Click Back
SettingsContent->>MonarchSettingsModal: onBack()
MonarchSettingsModal->>SettingsContent: clear detailView, slideDirection='backward'
SettingsContent->>PanelsDetails: render PreferencesPanel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
added
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@src/features/markets/components/market-settings-modal.tsx`:
- Line 216: The call to openModal('monarchSettings', { initialCategory: 'vaults'
}) passes an invalid SettingsCategory; replace the string 'vaults' with one of
the allowed values ('transaction', 'display', 'filters', 'preferences', or
'experimental') so the initialCategory prop on the monarchSettings modal is
valid — update the openModal invocation accordingly.
In `@src/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsx`:
- Around line 61-66: FilteredAvailableMarkets can shrink leaving currentPage out
of range which makes paginatedMarkets empty and shows "Page X of Y"; add logic
to clamp or reset currentPage whenever filteredAvailableMarkets (or totalPages)
changes. Specifically, compute totalPages = Math.max(1,
Math.ceil(filteredAvailableMarkets.length / ITEMS_PER_PAGE)) and in a useEffect
watching filteredAvailableMarkets or totalPages call setCurrentPage(prev =>
Math.min(prev, totalPages)) (or reset to 1 if totalPages === 1) so
paginatedMarkets and the displayed page count remain consistent; reference
variables/functions: filteredAvailableMarkets, ITEMS_PER_PAGE, totalPages,
paginatedMarkets, currentPage, and setCurrentPage.
In `@src/modals/settings/monarch-settings/details/RpcDetail.tsx`:
- Around line 27-44: The RPC validation fetch currently has no timeout; wrap the
fetch call with an AbortController, set a 10s timer that calls
controller.abort(), pass controller.signal into fetch(url, { ..., signal }), and
clear the timer after a successful response; also update the existing catch
block (the one handling errors from the fetch in RpcDetail.tsx / the RPC
validation function) to detect an abort (err.name === 'AbortError') and return
an { isValid: false, error: 'RPC request timed out' } result (or similar) so the
save button doesn't remain stuck loading.
In `@src/modals/settings/monarch-settings/details/TrendingDetail.tsx`:
- Around line 134-139: The IconSwitch used in TrendingDetail is missing an
accessible label; update the IconSwitch invocation (the component with props
selected={isEnabled} and onChange={setTrendingEnabled}) to include an aria-label
(or aria-labelledby) prop such as a descriptive string like "Enable trending" or
the appropriate i18n key so screen readers can identify the toggle; ensure the
prop is passed directly to IconSwitch so it lands on the underlying input
element.
- Around line 58-192: Add accessible labels to the threshold inputs by passing
descriptive aria-label props to the Input inside CompactInput and ensure each
instance call in TrendingDetail supplies different labels; update CompactInput
to accept an optional ariaLabel prop and forward it to the Input (function
CompactInput), then when rendering CompactInput in TrendingDetail for
supply/borrow pct/usd set ariaLabel values that include the time window label
and metric (e.g., "{label} supply flow percentage", "{label} supply flow USD",
"{label} borrow flow percentage", "{label} borrow flow USD") so screen readers
can uniquely identify each control.
In `@src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx`:
- Around line 205-210: The Morpho toggle button in TrustedVaultsDetail.tsx
currently toggles state via setMorphoSectionOpen but does not expose the
expanded state for assistive tech; update the button (the element using
onClick={() => setMorphoSectionOpen((prev) => !prev)}) to include aria-expanded
tied to the Morpho section state and add aria-controls pointing to the
collapsible panel’s id (create a stable id for the panel if missing) so screen
readers can detect expansion/collapse; ensure the collapsible panel element has
the matching id and appropriate role (e.g., region) if not already present.
- Around line 58-68: The fetch for Morpho vaults (useAllMorphoVaultsQuery)
currently only reads data and isLoading and maps morphoVaults into
morphoWhitelistedVaults, but doesn't handle query failures; extract isError from
useAllMorphoVaultsQuery and update the component's rendering logic to check
isError (and optionally use morphoLoading) before falling back to the
empty-state so that an explicit error message/UI is shown when the query fails;
adjust any places that rely on morphoWhitelistedVaults (e.g., the KnownVault
mapping) to handle the error path cleanly and avoid showing the empty-length
message on errors.
In `@src/modals/settings/monarch-settings/panels/DisplayPanel.tsx`:
- Around line 14-37: The toggle currently derives state from theme which fails
when theme === 'system'; update the component to use resolvedTheme from useTheme
to compute the current mode (replace isDarkMode = theme === 'dark' with
isDarkMode = resolvedTheme === 'dark') and change the IconSwitch onChange to
call setTheme(resolvedTheme === 'dark' ? 'light' : 'dark') so toggling always
flips the actual applied theme; reference useTheme, resolvedTheme, theme,
setTheme, isDarkMode, IconSwitch and mounted when making this change.
In `@src/modals/settings/monarch-settings/SettingsSidebar.tsx`:
- Around line 32-51: The category buttons in SettingsSidebar.tsx lose their
accessible name when collapsed because only the icon is visible; add an
accessible label by setting aria-label (or aria-labelledby) on the <button> to
the category name (use cat.label) when collapsed (or always) so screen readers
can announce the button; update the button element that renders Icon and checks
collapsed/isSelected to include aria-label={cat.label} (or aria-labelledby
pointing to the visible span id) and ensure this works with disabled and
isSelected props.
In `@src/stores/useModalStore.ts`:
- Around line 44-47: The modal type is incorrect:
monarchSettings.initialCategory currently allows 'vaults' and 'markets' but
SettingsCategory and PANEL_COMPONENTS (used in SettingsContent.tsx) only support
'transaction'|'display'|'filters'|'preferences'|'experimental', causing
market-settings-modal.tsx to open an unsupported panel; fix by making the types
consistent—either remove 'vaults' and 'markets' from
monarchSettings.initialCategory and update market-settings-modal.tsx to pass one
of the valid SettingsCategory values, or extend SettingsCategory and add
corresponding entries to PANEL_COMPONENTS (and any missing panel components) so
'vaults' and 'markets' are supported everywhere (ensure symbol names:
monarchSettings.initialCategory, SettingsCategory, PANEL_COMPONENTS,
SettingsContent.tsx, market-settings-modal.tsx are updated accordingly).
🧹 Nitpick comments (3)
src/modals/settings/monarch-settings/details/TrendingDetail.tsx (1)
99-117: Avoid O(n²) market lookup in the preview.
allMarkets.find(...)inside the loop scales poorly. Build a key→market map once.Suggested refactor
+ const marketByMetricsKey = useMemo(() => { + const map = new Map<string, Market>(); + for (const market of allMarkets) { + map.set(getMetricsKey(market.morphoBlue.chain.id, market.uniqueKey), market); + } + return map; + }, [allMarkets]); + const matchingMarkets = useMemo(() => { if (!isEnabled || metricsMap.size === 0) return []; const matches: Array<{ market: Market; supplyFlowPct1h: number }> = []; for (const [key, metrics] of metricsMap) { if (isMarketTrending(metrics, trendingConfig)) { - const market = allMarkets.find((m) => getMetricsKey(m.morphoBlue.chain.id, m.uniqueKey) === key); + const market = marketByMetricsKey.get(key); if (market) { matches.push({ market, supplyFlowPct1h: metrics.flows['1h']?.supplyFlowPct ?? 0, }); } } } return matches.sort((a, b) => (b.market.state?.supplyAssetsUsd ?? 0) - (a.market.state?.supplyAssetsUsd ?? 0)); - }, [isEnabled, metricsMap, trendingConfig, allMarkets]); + }, [isEnabled, metricsMap, trendingConfig, marketByMetricsKey]);src/modals/settings/monarch-settings/panels/PreferencesPanel.tsx (1)
64-68: Consider using semantic color tokens.The overflow indicator uses
bg-gray-200 dark:bg-gray-700while other parts of the UI use semantic tokens likebg-surfaceandtext-secondary. Minor consistency nit.src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx (1)
14-75: Reuse the shared vault key helper.
Local key format differs from the global helper; using the shared helper avoids drift and keeps dedupe logic consistent.♻️ Suggested change
-import { known_vaults, type KnownVault, type TrustedVault } from '@/constants/vaults/known_vaults'; +import { known_vaults, getVaultKey as buildVaultKey, type KnownVault, type TrustedVault } from '@/constants/vaults/known_vaults'; @@ -const getVaultKey = (v: KnownVault) => `${v.address.toLowerCase()}-${v.chainId}`; - -const monarchVaultKeys = useMemo(() => new Set(known_vaults.map(getVaultKey)), []); +const monarchVaultKeys = useMemo( + () => new Set(known_vaults.map((v) => buildVaultKey(v.address, v.chainId))), + [], +); @@ - const uniqueMorphoVaults = morphoWhitelistedVaults.filter((v) => !monarchVaultKeys.has(getVaultKey(v))); + const uniqueMorphoVaults = morphoWhitelistedVaults.filter( + (v) => !monarchVaultKeys.has(buildVaultKey(v.address, v.chainId)), + ); @@ - const uniqueMorphoVaults = morphoWhitelistedVaults.filter((v) => !monarchVaultKeys.has(getVaultKey(v))); + const uniqueMorphoVaults = morphoWhitelistedVaults.filter( + (v) => !monarchVaultKeys.has(buildVaultKey(v.address, v.chainId)), + );Also applies to: 106-107
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
app/settings/faq/page.tsxapp/settings/page.tsxsrc/components/layout/header/Navbar.tsxsrc/components/layout/header/NavbarMobile.tsxsrc/components/ui/palette-preview.tsxsrc/constants/chartColors.tssrc/features/market-detail/components/charts/chart-utils.tsxsrc/features/markets/components/market-settings-modal.tsxsrc/modals/registry.tsxsrc/modals/settings/blacklisted-markets-modal.tsxsrc/modals/settings/custom-rpc-settings.tsxsrc/modals/settings/monarch-settings/MonarchSettingsModal.tsxsrc/modals/settings/monarch-settings/SettingItem.tsxsrc/modals/settings/monarch-settings/SettingsContent.tsxsrc/modals/settings/monarch-settings/SettingsHeader.tsxsrc/modals/settings/monarch-settings/SettingsSidebar.tsxsrc/modals/settings/monarch-settings/constants.tssrc/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsxsrc/modals/settings/monarch-settings/details/RpcDetail.tsxsrc/modals/settings/monarch-settings/details/TrendingDetail.tsxsrc/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsxsrc/modals/settings/monarch-settings/details/index.tssrc/modals/settings/monarch-settings/panels/DisplayPanel.tsxsrc/modals/settings/monarch-settings/panels/ExperimentalPanel.tsxsrc/modals/settings/monarch-settings/panels/FiltersPanel.tsxsrc/modals/settings/monarch-settings/panels/PreferencesPanel.tsxsrc/modals/settings/monarch-settings/panels/TransactionPanel.tsxsrc/modals/settings/monarch-settings/panels/index.tssrc/modals/settings/trending-settings-modal.tsxsrc/modals/settings/trusted-vaults-modal.tsxsrc/stores/useModalStore.ts
💤 Files with no reviewable changes (6)
- src/modals/settings/blacklisted-markets-modal.tsx
- app/settings/page.tsx
- src/modals/settings/trusted-vaults-modal.tsx
- src/modals/settings/custom-rpc-settings.tsx
- src/modals/settings/trending-settings-modal.tsx
- app/settings/faq/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-12T09:23:16.495Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 63
File: app/markets/components/MarketRowDetail.tsx:49-52
Timestamp: 2024-10-12T09:23:16.495Z
Learning: When rendering oracle feeds in `ExpandedMarketDetail` (`app/markets/components/MarketRowDetail.tsx`), prefer explicit rendering over iterating keys when dealing with a small number of feeds.
Applied to files:
src/modals/settings/monarch-settings/details/TrendingDetail.tsxsrc/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsx
📚 Learning: 2025-12-09T10:06:43.810Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 231
File: src/hooks/useDeployMorphoMarketV1Adapter.ts:3-3
Timestamp: 2025-12-09T10:06:43.810Z
Learning: In Wagmi v3, useConnection is the correct hook to obtain the connected wallet address, chainId, and connection status (isConnected). This replaces the useAccount hook from Wagmi v2. Usage: const { address, chainId, isConnected } = useConnection() from 'wagmi'.
Applied to files:
src/components/layout/header/NavbarMobile.tsxsrc/components/layout/header/Navbar.tsx
📚 Learning: 2026-01-15T09:30:41.378Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 302
File: src/features/market-detail/components/charts/collateral-at-risk-chart.tsx:51-57
Timestamp: 2026-01-15T09:30:41.378Z
Learning: In Morpho Blue, oraclePrice is scaled by 1e36 and already accounts for decimal differences between collateral and loan assets. When computing collateral value in loan-asset terms, use (collateral * oraclePrice) / 10**36 (BigInt arithmetic as needed). Do not apply additional decimal adjustments for LTV/value calculations in code paths beyond display. The decimal adjustment in market-view.tsx should only affect display formatting.
Applied to files:
src/features/market-detail/components/charts/chart-utils.tsx
🧬 Code graph analysis (16)
src/modals/settings/monarch-settings/panels/TransactionPanel.tsx (4)
src/modals/settings/monarch-settings/constants.ts (1)
DetailView(8-8)src/stores/useAppSettings.ts (1)
useAppSettings(37-59)src/components/providers/CustomRpcProvider.tsx (1)
useCustomRpcContext(39-45)src/modals/settings/monarch-settings/SettingItem.tsx (2)
SettingToggleItem(20-53)SettingActionItem(63-83)
src/modals/settings/monarch-settings/details/RpcDetail.tsx (4)
src/modals/settings/monarch-settings/details/index.ts (1)
RpcDetail(4-4)src/components/providers/CustomRpcProvider.tsx (1)
useCustomRpcContext(39-45)src/hooks/useStyledToast.tsx (1)
useStyledToast(5-37)src/components/ui/spinner.tsx (1)
Spinner(22-58)
src/modals/settings/monarch-settings/MonarchSettingsModal.tsx (3)
src/modals/settings/monarch-settings/constants.ts (2)
SettingsCategory(6-6)DetailView(8-8)src/modals/settings/monarch-settings/SettingsSidebar.tsx (1)
SettingsSidebar(55-88)src/modals/settings/monarch-settings/SettingsContent.tsx (1)
SettingsContent(52-98)
src/modals/settings/monarch-settings/panels/ExperimentalPanel.tsx (4)
src/modals/settings/monarch-settings/constants.ts (1)
DetailView(8-8)src/modals/settings/monarch-settings/panels/index.ts (1)
ExperimentalPanel(5-5)src/stores/useMarketPreferences.ts (1)
useMarketPreferences(116-185)src/modals/settings/monarch-settings/SettingItem.tsx (2)
SettingToggleItem(20-53)SettingActionItem(63-83)
src/modals/settings/monarch-settings/SettingsSidebar.tsx (1)
src/modals/settings/monarch-settings/constants.ts (3)
SettingsCategory(6-6)CategoryConfig(10-15)SETTINGS_CATEGORIES(17-23)
src/modals/settings/monarch-settings/SettingItem.tsx (2)
src/components/ui/icon-switch.tsx (1)
IconSwitch(133-250)src/components/ui/button.tsx (1)
Button(107-107)
src/constants/chartColors.ts (1)
src/stores/useChartPalette.ts (1)
ChartPaletteName(4-4)
src/modals/settings/monarch-settings/SettingsHeader.tsx (1)
src/modals/settings/monarch-settings/constants.ts (2)
DetailView(8-8)DETAIL_TITLES(25-30)
src/modals/settings/monarch-settings/details/TrendingDetail.tsx (5)
src/stores/useMarketPreferences.ts (2)
TrendingWindowConfig(10-17)useMarketPreferences(116-185)src/utils/balance.ts (1)
formatReadable(25-49)src/hooks/queries/useMarketMetricsQuery.ts (3)
useMarketMetricsMap(169-185)isMarketTrending(201-247)getMetricsKey(60-60)src/hooks/useProcessedMarkets.ts (1)
useProcessedMarkets(28-91)src/utils/types.ts (1)
Market(287-334)
src/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsx (7)
src/hooks/useProcessedMarkets.ts (1)
useProcessedMarkets(28-91)src/stores/useBlacklistedMarkets.ts (1)
useBlacklistedMarkets(33-102)src/hooks/useStyledToast.tsx (1)
useStyledToast(5-37)src/utils/types.ts (1)
Market(287-334)src/utils/markets.ts (1)
blacklistedMarkets(21-25)src/features/markets/components/market-identity.tsx (1)
MarketIdentity(32-369)src/modals/settings/blacklisted-markets-modal.tsx (1)
BlacklistedMarketsModal(23-261)
src/components/layout/header/NavbarMobile.tsx (1)
src/hooks/useModal.ts (1)
useModal(25-90)
src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx (10)
src/constants/vaults/known_vaults.ts (4)
KnownVault(41-43)getVaultKey(384-384)known_vaults(62-378)TrustedVault(33-39)src/components/shared/network-icon.tsx (1)
NetworkIcon(4-15)src/features/autovault/components/vault-identity.tsx (1)
VaultIdentity(31-158)src/components/ui/icon-switch.tsx (1)
IconSwitch(133-250)src/modals/settings/monarch-settings/details/index.ts (1)
TrustedVaultsDetail(2-2)src/stores/useTrustedVaults.ts (1)
useTrustedVaults(36-76)src/hooks/queries/useAllMorphoVaultsQuery.ts (1)
useAllMorphoVaultsQuery(17-32)src/components/ui/button.tsx (1)
Button(107-107)src/components/ui/divider.tsx (1)
Divider(25-25)src/components/ui/spinner.tsx (1)
Spinner(22-58)
src/modals/registry.tsx (1)
src/modals/settings/monarch-settings/MonarchSettingsModal.tsx (1)
MonarchSettingsModal(15-86)
src/modals/settings/monarch-settings/panels/DisplayPanel.tsx (5)
src/stores/useAppSettings.ts (1)
useAppSettings(37-59)src/stores/useChartPalette.ts (1)
useChartPalette(18-36)src/components/ui/icon-switch.tsx (1)
IconSwitch(133-250)src/modals/settings/monarch-settings/SettingItem.tsx (1)
SettingToggleItem(20-53)src/components/ui/palette-preview.tsx (1)
PaletteSelector(58-77)
src/modals/settings/monarch-settings/panels/PreferencesPanel.tsx (5)
src/modals/settings/monarch-settings/constants.ts (1)
DetailView(8-8)src/stores/useTrustedVaults.ts (1)
useTrustedVaults(36-76)src/constants/vaults/known_vaults.ts (1)
defaultTrustedVaults(380-382)src/modals/settings/monarch-settings/SettingItem.tsx (1)
SettingActionItem(63-83)src/features/autovault/components/vault-identity.tsx (1)
VaultIdentity(31-158)
src/components/layout/header/Navbar.tsx (1)
src/hooks/useModal.ts (1)
useModal(25-90)
🔇 Additional comments (23)
src/constants/chartColors.ts (1)
113-117: PALETTE_META simplification looks consistent.Name-only metadata aligns with the updated UI usage.
src/features/market-detail/components/charts/chart-utils.tsx (1)
2-2: Type-only import is fine here.Keeps the type usage without adding runtime cost.
src/components/ui/palette-preview.tsx (1)
44-51: Compact styling update looks good.Smaller padding and text align with the modal layout.
src/modals/settings/monarch-settings/SettingItem.tsx (2)
20-52: SettingToggleItem looks solid.Layout and props wiring are clean.
63-82: SettingActionItem looks solid.Button wiring and layout are straightforward.
src/modals/settings/monarch-settings/constants.ts (1)
6-30: Constants are clear and centralized.Nice, tidy config surface.
src/modals/settings/monarch-settings/panels/FiltersPanel.tsx (1)
8-49: FiltersPanel wiring looks good.Toggles map cleanly to the store.
src/components/layout/header/Navbar.tsx (1)
17-17: LGTM!Clean integration with the modal system. The empty props object is valid since
initialCategoryis optional.Also applies to: 82-82, 191-191
src/components/layout/header/NavbarMobile.tsx (1)
145-148: LGTM!Properly closes the menu after opening the modal. Works as expected.
src/modals/settings/monarch-settings/panels/TransactionPanel.tsx (1)
1-56: LGTM!Clean panel implementation. Good use of optional chaining for the navigation callback and proper dark mode handling on the badge.
src/modals/settings/monarch-settings/panels/ExperimentalPanel.tsx (1)
1-36: LGTM!Consistent with the TransactionPanel pattern. Store bindings align with
useMarketPreferences.src/modals/settings/monarch-settings/details/index.ts (1)
1-4: LGTM. Clean barrel re-exports.src/modals/settings/monarch-settings/panels/index.ts (1)
1-5: LGTM. Straightforward re-exports.src/modals/registry.tsx (1)
31-34: LGTM!Lazy import and registry entry follow the established patterns in this file. Consolidating the separate settings modals into a single
monarchSettingsentry is a clean approach.Also applies to: 53-53
src/modals/settings/monarch-settings/SettingsHeader.tsx (1)
12-39: LGTM!Clean implementation with proper accessibility labels on interactive elements.
src/modals/settings/monarch-settings/details/RpcDetail.tsx (2)
72-127: LGTM!Save handler properly validates before persisting, and provides clear feedback to the user about needing to refresh.
153-267: LGTM!UI is well structured with clear visual feedback for custom RPC status.
src/modals/settings/monarch-settings/panels/PreferencesPanel.tsx (1)
14-34: LGTM!Good use of mounted state for hydration safety. Memoization of vault keys and sorting is appropriate.
src/modals/settings/monarch-settings/MonarchSettingsModal.tsx (2)
15-53: LGTM!State management is clean. The pattern of resetting state on modal open and managing navigation between category and detail views is well structured.
55-85: LGTM!Modal composition with sidebar and content panes is straightforward. Responsive height constraints are reasonable.
src/modals/settings/monarch-settings/SettingsContent.tsx (2)
13-41: Clear panel/detail registry.
The mappings keep the render surface predictable and easy to extend.
52-95: Clean split between panel and detail rendering.
The conditional layout stays straightforward and isolated.src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx (1)
24-47: Vault row composition looks solid.
Clear layout and toggle wiring.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/modals/settings/monarch-settings/details/BlacklistedMarketsDetail.tsx
Show resolved
Hide resolved
Comment on lines
+27
to
+44
| try { | ||
| const response = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| id: 1, | ||
| jsonrpc: '2.0', | ||
| method: 'eth_chainId', | ||
| params: [], | ||
| }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| return { | ||
| isValid: false, | ||
| error: `RPC server responded with ${response.status}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing timeout on RPC validation fetch.
If the RPC endpoint is unresponsive, this fetch will hang indefinitely, freezing the save button in loading state. Add an AbortController with a reasonable timeout (e.g., 10 seconds).
Proposed fix
try {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 10000);
+
const response = await fetch(url, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
id: 1,
jsonrpc: '2.0',
method: 'eth_chainId',
params: [],
}),
+ signal: controller.signal,
});
+ clearTimeout(timeoutId);
if (!response.ok) {And update the catch block to handle abort:
} catch (error) {
+ if (error instanceof Error && error.name === 'AbortError') {
+ return { isValid: false, error: 'RPC validation timed out' };
+ }
return {
isValid: false,
error: `Failed to connect to RPC: ${error instanceof Error ? error.message : 'Network error'}`,
};
}🤖 Prompt for AI Agents
In `@src/modals/settings/monarch-settings/details/RpcDetail.tsx` around lines 27 -
44, The RPC validation fetch currently has no timeout; wrap the fetch call with
an AbortController, set a 10s timer that calls controller.abort(), pass
controller.signal into fetch(url, { ..., signal }), and clear the timer after a
successful response; also update the existing catch block (the one handling
errors from the fetch in RpcDetail.tsx / the RPC validation function) to detect
an abort (err.name === 'AbortError') and return an { isValid: false, error: 'RPC
request timed out' } result (or similar) so the save button doesn't remain stuck
loading.
Comment on lines
+58
to
+68
| const { data: morphoVaults = [], isLoading: morphoLoading } = useAllMorphoVaultsQuery(); | ||
|
|
||
| const morphoWhitelistedVaults = useMemo<KnownVault[]>(() => { | ||
| return morphoVaults.map((vault) => ({ | ||
| address: vault.address as `0x${string}`, | ||
| chainId: vault.chainId, | ||
| name: vault.name, | ||
| curator: 'unknown', | ||
| asset: vault.assetAddress as `0x${string}`, | ||
| })); | ||
| }, [morphoVaults]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the hook definition to verify it returns isError
rg -A 5 "useAllMorphoVaultsQuery" --type tsx --type ts | head -40Repository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
# Read the file around the lines mentioned in the review
cat -n src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx | sed -n '50,75p'Repository: antoncoding/monarch
Length of output: 1351
🏁 Script executed:
# Also check the lines around 214-223
cat -n src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx | sed -n '210,230p'Repository: antoncoding/monarch
Length of output: 1234
🏁 Script executed:
# Search for useAllMorphoVaultsQuery definition
rg "useAllMorphoVaultsQuery" --type js --type ts -A 10Repository: antoncoding/monarch
Length of output: 5072
🏁 Script executed:
# Also try a broader search
rg "export.*useAllMorphoVaultsQuery" -A 10Repository: antoncoding/monarch
Length of output: 948
🏁 Script executed:
cat -n src/hooks/queries/useAllMorphoVaultsQuery.tsRepository: antoncoding/monarch
Length of output: 1395
🏁 Script executed:
# Check the full conditional block to understand the logic flow
cat -n src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx | sed -n '210,240p'Repository: antoncoding/monarch
Length of output: 1452
Add explicit error handling for failed Morpho vault fetches.
When the query fails, it falls into the empty-state message, which misleads users. Extract isError from the hook and check it before the empty-length condition to show an appropriate error message.
🐛 Suggested change
- const { data: morphoVaults = [], isLoading: morphoLoading } = useAllMorphoVaultsQuery();
+ const { data: morphoVaults = [], isLoading: morphoLoading, isError: morphoError } = useAllMorphoVaultsQuery();
@@
- ) : sortedMorphoVaults.length === 0 ? (
+ ) : morphoError ? (
+ <div className="py-4 text-center text-xs text-secondary">
+ Unable to load Morpho vaults. Please try again.
+ </div>
+ ) : sortedMorphoVaults.length === 0 ? (Also applies to: 214-223
🤖 Prompt for AI Agents
In `@src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx` around
lines 58 - 68, The fetch for Morpho vaults (useAllMorphoVaultsQuery) currently
only reads data and isLoading and maps morphoVaults into
morphoWhitelistedVaults, but doesn't handle query failures; extract isError from
useAllMorphoVaultsQuery and update the component's rendering logic to check
isError (and optionally use morphoLoading) before falling back to the
empty-state so that an explicit error message/UI is shown when the query fails;
adjust any places that rely on morphoWhitelistedVaults (e.g., the KnownVault
mapping) to handle the error path cleanly and avoid showing the empty-length
message on errors.
Comment on lines
+205
to
+210
| <div className="flex flex-col gap-3 rounded bg-surface-soft p-3"> | ||
| <button | ||
| type="button" | ||
| className="flex items-center justify-between text-left text-sm font-medium text-primary" | ||
| onClick={() => setMorphoSectionOpen((prev) => !prev)} | ||
| > |
There was a problem hiding this comment.
Expose the expanded state on the Morpho toggle button.
This helps screen readers understand the collapsible section.
♿ Suggested change
<button
type="button"
className="flex items-center justify-between text-left text-sm font-medium text-primary"
onClick={() => setMorphoSectionOpen((prev) => !prev)}
+ aria-expanded={morphoSectionOpen}
>📝 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.
Suggested change
| <div className="flex flex-col gap-3 rounded bg-surface-soft p-3"> | |
| <button | |
| type="button" | |
| className="flex items-center justify-between text-left text-sm font-medium text-primary" | |
| onClick={() => setMorphoSectionOpen((prev) => !prev)} | |
| > | |
| <div className="flex flex-col gap-3 rounded bg-surface-soft p-3"> | |
| <button | |
| type="button" | |
| className="flex items-center justify-between text-left text-sm font-medium text-primary" | |
| onClick={() => setMorphoSectionOpen((prev) => !prev)} | |
| aria-expanded={morphoSectionOpen} | |
| > |
🤖 Prompt for AI Agents
In `@src/modals/settings/monarch-settings/details/TrustedVaultsDetail.tsx` around
lines 205 - 210, The Morpho toggle button in TrustedVaultsDetail.tsx currently
toggles state via setMorphoSectionOpen but does not expose the expanded state
for assistive tech; update the button (the element using onClick={() =>
setMorphoSectionOpen((prev) => !prev)}) to include aria-expanded tied to the
Morpho section state and add aria-controls pointing to the collapsible panel’s
id (create a stable id for the panel if missing) so screen readers can detect
expansion/collapse; ensure the collapsible panel element has the matching id and
appropriate role (e.g., region) if not already present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Put settings in a modal so we can
Summary by CodeRabbit
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.