Skip to content

feat: reallocation modal optimization#167

Merged
antoncoding merged 6 commits intomasterfrom
feat/reallocate
Oct 25, 2025
Merged

feat: reallocation modal optimization#167
antoncoding merged 6 commits intomasterfrom
feat/reallocate

Conversation

@antoncoding
Copy link
Owner

@antoncoding antoncoding commented Oct 25, 2025

Features

  • Filtering on smaller markets
  • Allow adjust market settings

Summary by CodeRabbit

  • New Features

    • Added search functionality to filter markets by symbol or ID.
    • Introduced market settings modal for configurable filters: hide small markets, include unknown tokens, show oracle settings, and entries per page.
    • Markets now display with persistent user preferences stored locally.
  • Bug Fixes

    • Improved single-select flow in market modals for faster selection.
  • Refactor

    • Updated internal storage key management for market settings.
    • Enhanced token tracking with source annotations.

@vercel
Copy link
Contributor

vercel bot commented Oct 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
monarch Ready Ready Preview Comment Oct 25, 2025 7:19am

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2025

📝 Walkthrough

Walkthrough

This PR introduces token source tracking across the app and enhances market display with settings, search, and filtering capabilities. It refactors modal components to use HeroUI's Modal suite, updates storage key constants for consistency, and adjusts component APIs to support the new functionality.

Changes

Cohort / File(s) Summary
Token Source Annotation
src/utils/tokens.ts, src/components/providers/TokenProvider.tsx, app/markets/components/markets.tsx
Adds optional source field ('local' | 'external' | 'unknown') to token types. TokenProvider now tracks token origin by annotating supportedTokens with 'local' source and Pendle assets with 'external' source. markets.tsx adds 'unknown' source to unknown token entries.
Market Settings & Constants
src/constants/markets.ts, src/utils/storageKeys.ts, app/markets/components/MarketSettingsModal.tsx
Adds DEFAULT_MIN_SUPPLY_USD constant (1000). Replaces old storage key constants with MarketsShowSmallMarkets, MarketsShowUnknownTokens, MarketsShowUnknownOracle. MarketSettingsModal adds custom classNames prop for z-index layering.
Market Selection & Display
src/components/common/MarketSelectionModal.tsx, src/components/common/MarketsTableWithSameLoanAsset.tsx
MarketSelectionModal refactored to use HeroUI Modal, adds isOpen prop, simplifies single-select flow. MarketsTableWithSameLoanAsset gains search input, settings modal, persistent filter state (hideSmallMarkets, unknownTokens, unknownOracle, minSupply), and pagination adjustments. Adds showSettings prop.
Position Modal API
app/positions/components/RebalanceModal.tsx
Removes PER_PAGE export. Updates RebalanceModalProps to require onClose and refetch callbacks.
Market Hook
src/hooks/useVaultAllocations.ts
Changes useMarkets() to destructure allMarkets instead of markets; updates all market lookups accordingly.
Config
.gitignore
Adds CLAUDE.md to ignore list.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MarketsTable as MarketsTableWithSameLoanAsset
    participant Modal as MarketSettingsModal
    participant Storage as localStorage
    participant Provider as TokenProvider

    User->>MarketsTable: Search/Filter markets
    MarketsTable->>MarketsTable: Apply search query & filters
    MarketsTable->>MarketsTable: Compute effectiveMinSupply

    User->>MarketsTable: Click Settings
    MarketsTable->>Modal: Open with current settings
    
    User->>Modal: Adjust filters (minSupply, unknownTokens, etc.)
    Modal->>Storage: Save settings
    Storage-->>Modal: Persisted
    
    Modal->>MarketsTable: onSettingsChange
    MarketsTable->>MarketsTable: Re-filter & re-paginate
    MarketsTable-->>User: Updated market list

    Note over Provider: TokenProvider annotates<br/>tokens with source: local/external/unknown
    MarketsTable->>Provider: availableCollaterals
    Provider-->>MarketsTable: Tokens with source field
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Extra attention areas:
    • MarketsTableWithSameLoanAsset.tsx — significant logic expansion with search, filtering, pagination, and settings integration; verify filter logic and state management
    • Token source propagation across TokenProvider.tsx, markets.tsx, and tokens.ts — ensure source annotations are consistent
    • Storage key migration in storageKeys.ts — verify no orphaned references to old key names remain

Possibly related PRs

Poem

🎯 Tokens gain their source,
Markets search and filter flow,
Settings stick around,
✨ Clean modals, UI aglow.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The PR title "feat: reallocation modal optimization" refers to changes in RebalanceModal (prop updates and export removal), which are real but represent a minor portion of the changeset. However, this title misses the primary objectives stated in the PR description: adding filtering for smaller markets and implementing market settings UI. The bulk of the changes involve MarketsTableWithSameLoanAsset (search, filters, settings), MarketSettingsModal (new component), and supporting infrastructure. Someone scanning the commit history based on this title would expect to find reallocation modal improvements, not market filtering and settings features. Consider updating the title to reflect the main features: something like "feat: add market filtering and settings UI" or "feat: market filtering with configurable settings" would better represent the actual scope. This would make the PR's intent clear to reviewers scanning the history.
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/reallocate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (4)
src/components/common/MarketsTableWithSameLoanAsset.tsx (1)

56-67: Extract utility function to src/utils/.

Per coding guidelines, utility functions should be extracted from components to src/utils/. This pure function would fit well in a markets utility file.

As per coding guidelines

app/markets/components/MarketSettingsModal.tsx (1)

104-113: Z-index overrides: confirm API, consider centralizing values

Looks good for resolving layering. Please confirm @heroui/react Modal supports classNames.wrapper/backdrop in your version. Consider replacing magic numbers with a shared z-index token to avoid future collisions (toasts, drawers, wallet modals).

app/markets/components/markets.tsx (2)

92-95: Default min supply now '1000': add a one-time migration for existing users

Users with an existing MarketsUsdMinSupplyKey of '' (or '0') won’t pick up the new default. Add a small effect to backfill to 1000 when the stored value is empty; also ensure applyFilterAndSort treats '' as 0 to avoid accidental filtering.

Example:

useEffect(() => {
  if (usdMinSupply === '') setUsdMinSupply(DEFAULT_MIN_SUPPLY_USD.toString());
  // optionally: if (usdMinSupply === '0') setUsdMinSupply(DEFAULT_MIN_SUPPLY_USD.toString());
  // eslint-disable-next-line react-hooks/exhaustive-deps
}, []); // run once

143-144: Type-safety for token source annotation

Adding source: 'unknown' is aligned with the new token model. Confirm UnknownERC20Token (and related unions) include source?: 'local' | 'external' | 'unknown'. If inference widens, use a literal:

source: 'unknown' as const,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 887b8a5 and 9698238.

📒 Files selected for processing (10)
  • CLAUDE.md (1 hunks)
  • app/markets/components/MarketSettingsModal.tsx (1 hunks)
  • app/markets/components/markets.tsx (3 hunks)
  • app/positions/components/RebalanceModal.tsx (1 hunks)
  • src/components/common/MarketSelectionModal.tsx (5 hunks)
  • src/components/common/MarketsTableWithSameLoanAsset.tsx (10 hunks)
  • src/components/providers/TokenProvider.tsx (3 hunks)
  • src/constants/markets.ts (1 hunks)
  • src/hooks/useVaultAllocations.ts (3 hunks)
  • src/utils/tokens.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
src/components/**

📄 CodeRabbit inference engine (AGENTS.md)

Place UI components under src/components/

Files:

  • src/components/providers/TokenProvider.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
**/*.{ts,tsx,js,jsx,css,md,json,yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

Run Prettier via pnpm format with the repository profile (100-char width, 2-space indent, single quotes, trailing commas, Tailwind-aware ordering)

Files:

  • src/components/providers/TokenProvider.tsx
  • src/utils/tokens.ts
  • src/constants/markets.ts
  • app/markets/components/MarketSettingsModal.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
  • src/hooks/useVaultAllocations.ts
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
  • CLAUDE.md
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Adhere to ESLint (Airbnb + Next.js) rules for hook safety and import hygiene

Files:

  • src/components/providers/TokenProvider.tsx
  • src/utils/tokens.ts
  • src/constants/markets.ts
  • app/markets/components/MarketSettingsModal.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
  • src/hooks/useVaultAllocations.ts
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
src/components/**/[A-Z][A-Za-z0-9]*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase filenames for React components (e.g., VaultBanner.tsx)

Files:

  • src/components/providers/TokenProvider.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
{app,src/components}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Keep Tailwind class lists purposeful and lean; consolidate repeating patterns with tailwind-merge

Files:

  • src/components/providers/TokenProvider.tsx
  • app/markets/components/MarketSettingsModal.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
{app,src}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

When using the shared Spinner component, pass numeric pixel values to size (e.g., size={12}), not semantic strings

Files:

  • src/components/providers/TokenProvider.tsx
  • app/markets/components/MarketSettingsModal.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not define utility/business logic inside components; extract to src/utils/

Files:

  • src/components/providers/TokenProvider.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
{src/hooks/**/*.ts,src/hooks/**/*.tsx,src/components/**/*.tsx,app/**/*.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Memoize arrays/objects passed to hooks that trigger network requests (or pass a stable key) to avoid unnecessary re-fetches and effect retriggers

Files:

  • src/components/providers/TokenProvider.tsx
  • app/markets/components/MarketSettingsModal.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
  • src/hooks/useVaultAllocations.ts
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
src/utils/**

📄 CodeRabbit inference engine (AGENTS.md)

Place reusable utility functions under src/utils/

Files:

  • src/utils/tokens.ts
src/utils/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/utils/**/*.{ts,tsx}: Use camelCase for helper function names (e.g., formatApr)
Keep utilities pure and reusable; host business/utility logic in src/utils/

Files:

  • src/utils/tokens.ts
src/constants/**

📄 CodeRabbit inference engine (AGENTS.md)

Place shared constants under src/constants/

Files:

  • src/constants/markets.ts
src/constants/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use SCREAMING_SNAKE_CASE for shared constants

Files:

  • src/constants/markets.ts
app/**

📄 CodeRabbit inference engine (AGENTS.md)

Place Next.js route files under app/

Files:

  • app/markets/components/MarketSettingsModal.tsx
  • app/markets/components/markets.tsx
  • app/positions/components/RebalanceModal.tsx
src/hooks/**

📄 CodeRabbit inference engine (AGENTS.md)

Place React hooks under src/hooks/

Files:

  • src/hooks/useVaultAllocations.ts
src/hooks/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not define utility/business logic inside hooks; extract to src/utils/

Files:

  • src/hooks/useVaultAllocations.ts
src/hooks/use*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

For new on-chain hooks, mirror useERC20Approval/useTransactionWithToast patterns: compute context early, reuse useTransactionWithToast, and expose a minimal surface ({ action, isLoading }) with refetch callbacks

Files:

  • src/hooks/useVaultAllocations.ts
🧬 Code graph analysis (4)
src/components/providers/TokenProvider.tsx (1)
src/utils/tokens.ts (1)
  • ERC20Token (19-32)
src/components/common/MarketSelectionModal.tsx (3)
src/utils/types.ts (1)
  • Market (268-315)
src/components/common/Spinner.tsx (1)
  • Spinner (24-57)
src/components/common/MarketsTableWithSameLoanAsset.tsx (1)
  • MarketsTableWithSameLoanAsset (456-866)
src/components/common/MarketsTableWithSameLoanAsset.tsx (7)
src/constants/markets.ts (1)
  • DEFAULT_MIN_SUPPLY_USD (1-1)
src/contexts/MarketsContext.tsx (1)
  • useMarkets (265-271)
src/components/providers/TokenProvider.tsx (1)
  • useTokens (139-145)
src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (4-78)
src/components/Input/Input.tsx (1)
  • Input (17-97)
src/components/common/Button.tsx (1)
  • Button (4-54)
app/markets/components/MarketSettingsModal.tsx (1)
  • MarketSettingsModal (59-306)
app/markets/components/markets.tsx (2)
src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (4-78)
src/constants/markets.ts (1)
  • DEFAULT_MIN_SUPPLY_USD (1-1)
🔇 Additional comments (9)
src/constants/markets.ts (1)

1-1: LGTM!

Clean constant definition following the SCREAMING_SNAKE_CASE convention.

src/hooks/useVaultAllocations.ts (1)

45-45: LGTM!

Consistent rename to allMarkets.

src/utils/tokens.ts (1)

4-4: LGTM!

Clean addition of token source tracking with optional fields for backward compatibility.

Also applies to: 28-28, 40-40

src/components/providers/TokenProvider.tsx (1)

26-29: LGTM!

Clean implementation of token source tracking—local and external tokens properly annotated.

Also applies to: 58-58, 63-63, 94-94

app/positions/components/RebalanceModal.tsx (1)

20-26: LGTM!

Clean prop API update with explicit onClose and refetch callbacks.

Also applies to: 262-264, 273-276, 283-283

src/components/common/MarketsTableWithSameLoanAsset.tsx (2)

42-45: LGTM!

Clean settings system with configurable storage keys allowing multiple independent instances.

Also applies to: 464-490


504-552: LGTM!

Source-based sorting prioritizes local tokens and properly annotates unknown tokens.

src/components/common/MarketSelectionModal.tsx (1)

2-2: LGTM!

Clean refactor to use the HeroUI Modal component with improved prop API and simplified single-select flow.

Also applies to: 18-18, 23-23, 37-37, 41-41, 70-76, 107-184

app/markets/components/markets.tsx (1)

14-14: Using a single source of truth for the default is good

Importing DEFAULT_MIN_SUPPLY_USD keeps UI and logic consistent.

@antoncoding antoncoding merged commit 9cd4fa8 into master Oct 25, 2025
3 of 4 checks passed
@antoncoding antoncoding deleted the feat/reallocate branch October 25, 2025 07:20
Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
src/components/common/MarketSelectionModal.tsx (1)

109-116: Consider reviewing z-index strategy.

The z-index values (2200, 2190) are very high. MarketSettingsModal uses even higher values (2300, 2290), suggesting intentional modal stacking. While this works, high z-index values can cause stacking context issues and make the UI harder to maintain.

Consider whether nested modals are necessary or if there's a simpler approach (e.g., closing one modal before opening another, or using a single modal with different views).

src/components/common/MarketsTableWithSameLoanAsset.tsx (1)

504-508: Consider explicit handling of optional source field.

The sorting logic accesses the optional source field without explicit null handling. While this works (undefined tokens are treated as non-local), it's implicit and could be clearer:

-        (a.source === 'local' ? 0 : 1) - (b.source === 'local' ? 0 : 1) ||
+        ((a.source ?? 'unknown') === 'local' ? 0 : 1) - ((b.source ?? 'unknown') === 'local' ? 0 : 1) ||

This makes the fallback behavior explicit.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9698238 and 220df8a.

📒 Files selected for processing (6)
  • .gitignore (1 hunks)
  • app/markets/components/markets.tsx (3 hunks)
  • src/components/common/MarketSelectionModal.tsx (5 hunks)
  • src/components/common/MarketsTableWithSameLoanAsset.tsx (10 hunks)
  • src/hooks/useVaultAllocations.ts (3 hunks)
  • src/utils/storageKeys.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/hooks/useVaultAllocations.ts
🧰 Additional context used
📓 Path-based instructions (11)
app/**

📄 CodeRabbit inference engine (AGENTS.md)

Place Next.js route files under app/

Files:

  • app/markets/components/markets.tsx
**/*.{ts,tsx,js,jsx,css,md,json,yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

Run Prettier via pnpm format with the repository profile (100-char width, 2-space indent, single quotes, trailing commas, Tailwind-aware ordering)

Files:

  • app/markets/components/markets.tsx
  • src/utils/storageKeys.ts
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Adhere to ESLint (Airbnb + Next.js) rules for hook safety and import hygiene

Files:

  • app/markets/components/markets.tsx
  • src/utils/storageKeys.ts
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
{app,src/components}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Keep Tailwind class lists purposeful and lean; consolidate repeating patterns with tailwind-merge

Files:

  • app/markets/components/markets.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
{app,src}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

When using the shared Spinner component, pass numeric pixel values to size (e.g., size={12}), not semantic strings

Files:

  • app/markets/components/markets.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
{src/hooks/**/*.ts,src/hooks/**/*.tsx,src/components/**/*.tsx,app/**/*.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Memoize arrays/objects passed to hooks that trigger network requests (or pass a stable key) to avoid unnecessary re-fetches and effect retriggers

Files:

  • app/markets/components/markets.tsx
  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
src/utils/**

📄 CodeRabbit inference engine (AGENTS.md)

Place reusable utility functions under src/utils/

Files:

  • src/utils/storageKeys.ts
src/utils/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/utils/**/*.{ts,tsx}: Use camelCase for helper function names (e.g., formatApr)
Keep utilities pure and reusable; host business/utility logic in src/utils/

Files:

  • src/utils/storageKeys.ts
src/components/**

📄 CodeRabbit inference engine (AGENTS.md)

Place UI components under src/components/

Files:

  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
src/components/**/[A-Z][A-Za-z0-9]*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase filenames for React components (e.g., VaultBanner.tsx)

Files:

  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not define utility/business logic inside components; extract to src/utils/

Files:

  • src/components/common/MarketSelectionModal.tsx
  • src/components/common/MarketsTableWithSameLoanAsset.tsx
🧬 Code graph analysis (3)
app/markets/components/markets.tsx (2)
src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (4-78)
src/constants/markets.ts (1)
  • DEFAULT_MIN_SUPPLY_USD (1-1)
src/components/common/MarketSelectionModal.tsx (3)
src/components/common/Spinner.tsx (1)
  • Spinner (24-57)
src/components/common/MarketsTableWithSameLoanAsset.tsx (1)
  • MarketsTableWithSameLoanAsset (455-862)
src/components/common/Button.tsx (1)
  • Button (4-54)
src/components/common/MarketsTableWithSameLoanAsset.tsx (7)
src/constants/markets.ts (1)
  • DEFAULT_MIN_SUPPLY_USD (1-1)
src/contexts/MarketsContext.tsx (1)
  • useMarkets (265-271)
src/components/providers/TokenProvider.tsx (1)
  • useTokens (139-145)
src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (4-78)
src/components/Input/Input.tsx (1)
  • Input (17-97)
src/components/common/Button.tsx (1)
  • Button (4-54)
app/markets/components/MarketSettingsModal.tsx (1)
  • MarketSettingsModal (59-306)
🔇 Additional comments (9)
app/markets/components/markets.tsx (3)

85-88: Storage key usage is correct.

The migration to constants is good practice. However, ensure the storage key naming inconsistency flagged in src/utils/storageKeys.ts is addressed to prevent confusion.

Based on learnings from storageKeys review.


92-95: LGTM!

The default initialization with DEFAULT_MIN_SUPPLY_USD.toString() provides a sensible default value (1000 USD) instead of an empty string.


143-143: LGTM!

The source: 'unknown' field correctly implements the token source tracking feature introduced in this PR.

src/components/common/MarketSelectionModal.tsx (2)

66-74: Improved single-select flow.

Removing the setTimeout workaround and calling onClose() immediately is cleaner. The HeroUI Modal handles state transitions internally, so no artificial delay is needed.


104-180: LGTM!

The refactor to use HeroUI's Modal suite improves code clarity and maintainability. The structured approach with ModalHeader, ModalBody, and ModalFooter is more declarative and follows best practices.

src/components/common/MarketsTableWithSameLoanAsset.tsx (4)

55-66: LGTM!

The getMinSupplyThreshold helper provides robust parsing with proper fallback to DEFAULT_MIN_SUPPLY_USD and prevents negative values.


571-592: Filter logic is correct.

The search, whitelist, and small markets filters are implemented correctly. The logic properly handles edge cases (empty queries, zero thresholds).

Note: The small markets filter logic is correct but depends on resolving the hideSmallMarkets vs MarketsShowSmallMarkets naming mismatch flagged earlier.


722-759: LGTM!

The search and controls UI is clean and follows HeroUI patterns. The conditional rendering of the settings button based on showSettings prop is good practice.


845-859: Settings modal integration is correct.

The conditional rendering and prop wiring for MarketSettingsModal is properly implemented.

Note: Ensure the usdFilters storage pattern issue flagged earlier is resolved for consistency across the codebase.

const [searchQuery, setSearchQuery] = useState('');

// Settings state (persisted with storage key namespace)
const [hideSmallMarkets, setHideSmallMarkets] = useLocalStorage(keys.MarketsShowSmallMarkets, true);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Semantic mismatch between variable and storage key.

The variable is named hideSmallMarkets but the storage key is MarketsShowSmallMarkets. This creates confusion about the intended behavior. With default: true, markets are hidden by default, but the key name suggests they should be shown.

Either rename the variable to showSmallMarkets and invert the logic throughout, or use a storage key that matches "hide" semantics.

🤖 Prompt for AI Agents
In src/components/common/MarketsTableWithSameLoanAsset.tsx around line 481, the
variable hideSmallMarkets conflicts with the storage key
MarketsShowSmallMarkets; rename the state to showSmallMarkets and its setter to
setShowSmallMarkets, keep the storage key as-is, set the default to true
(meaning small markets are shown by default), and invert all usages/conditions
and any logic that previously relied on hideSmallMarkets (e.g., replace checks
like if (hideSmallMarkets) with if (!showSmallMarkets>?) — ensure the setter
calls are renamed accordingly so the semantics consistently reflect "show"
throughout the file.

Comment on lines +485 to +488
const [usdFilters, setUsdFilters] = useLocalStorage(keys.MarketsUsdMinSupplyKey, {
minSupply: DEFAULT_MIN_SUPPLY_USD.toString(),
minBorrow: '',
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent storage pattern for USD filters.

Using MarketsUsdMinSupplyKey to store an object containing both minSupply and minBorrow is inconsistent with:

  1. The separate MarketsUsdMinBorrowKey constant that exists but isn't used here
  2. How it's done in app/markets/components/markets.tsx (lines 92-96), where they're stored separately

This creates confusion about which pattern to follow. Either use separate keys for each value, or consolidate to a single key for the object across all files.

Comment on lines +17 to +19
export const MarketsShowSmallMarkets = 'monarch_show_small_markets'
export const MarketsShowUnknownTokens = 'includeUnknownTokens';
export const MarketsShowUnknownOracle = 'showUnknownOracle'; No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent naming convention across storage keys.

Line 17 follows the established pattern (monarch_ prefix + snake_case), but lines 18-19 use camelCase without the prefix. This inconsistency can confuse developers and makes the codebase harder to maintain.

Apply this diff to align with the established convention:

-export const MarketsShowSmallMarkets = 'monarch_show_small_markets'
-export const MarketsShowUnknownTokens = 'includeUnknownTokens';
-export const MarketsShowUnknownOracle = 'showUnknownOracle';
+export const MarketsShowSmallMarkets = 'monarch_show_small_markets';
+export const MarketsShowUnknownTokens = 'monarch_show_unknown_tokens';
+export const MarketsShowUnknownOracle = 'monarch_show_unknown_oracle';
📝 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
export const MarketsShowSmallMarkets = 'monarch_show_small_markets'
export const MarketsShowUnknownTokens = 'includeUnknownTokens';
export const MarketsShowUnknownOracle = 'showUnknownOracle';
export const MarketsShowSmallMarkets = 'monarch_show_small_markets';
export const MarketsShowUnknownTokens = 'monarch_show_unknown_tokens';
export const MarketsShowUnknownOracle = 'monarch_show_unknown_oracle';
🤖 Prompt for AI Agents
In src/utils/storageKeys.ts around lines 17 to 19, the storage key constants use
inconsistent naming (lines 18-19 are camelCase and missing the "monarch_"
prefix). Update those constants so their string values follow the established
convention: use the "monarch_" prefix and snake_case (e.g., change
'includeUnknownTokens' and 'showUnknownOracle' to
'monarch_include_unknown_tokens' and 'monarch_show_unknown_oracle'), keeping the
exported constant identifiers as-is or rename them consistently if desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant