Skip to content

fix: star page reset#203

Merged
antoncoding merged 1 commit intomasterfrom
fix/star-sort
Nov 30, 2025
Merged

fix: star page reset#203
antoncoding merged 1 commit intomasterfrom
fix/star-sort

Conversation

@antoncoding
Copy link
Copy Markdown
Owner

@antoncoding antoncoding commented Nov 30, 2025

fix #188

Summary by CodeRabbit

  • Bug Fixes
    • Improved pagination behavior: page no longer resets when sorting or starring items, only when applying filters are changed.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Nov 30, 2025

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

Project Deployment Preview Comments Updated (UTC)
monarch Ready Ready Preview Comment Nov 30, 2025 11:31pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 30, 2025

📝 Walkthrough

Walkthrough

The change modifies pagination behavior in the markets component. It removes automatic page reset from the sort/filter function and adds a targeted effect that resets the page only when filter parameters change, preventing unwanted resets when sorting or starring.

Changes

Cohort / File(s) Summary
Pagination Reset Logic
app/markets/components/markets.tsx
Removed unconditional page reset from applyFiltersAndSort function. Added new effect hook that resets pagination page specifically when filter state changes (network, tokens, oracle, USD filters, thresholds, trusted vaults, etc.), but not on sorting or starring updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify effect dependencies list is complete and correct (no missing filter parameters)
  • Confirm sorting still functions without triggering page reset
  • Check that filter updates still reset page as intended
  • Validate no edge cases introduced in conditional page reset logic

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title directly references the main issue being fixed (page reset when starring), matching the primary change in the code.
Linked Issues check ✅ Passed Code changes address issue #188 by removing unconditional page resets from sort/star actions and adding targeted reset only on filter changes, preserving pagination during starring while allowing sort updates.
Out of Scope Changes check ✅ Passed All changes focus on pagination behavior related to starring and filtering, directly scoped to issue #188 with no unrelated modifications.
✨ 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 fix/star-sort

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
Copy Markdown

@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: 0

🧹 Nitpick comments (1)
app/markets/components/markets.tsx (1)

396-413: Filter‑driven page reset looks right; double‑check dependency coverage

This hook nicely decouples pagination reset from sorting/starring and should address the “starring jumps back to page 1” issue.

One thing to confirm: are there any other knobs you consider “filters” that should also reset the page, e.g. showUnwhitelistedMarkets or changes to the trusted vaults list (userTrustedVaults / hasTrustedVault)? Right now, toggling those will keep the current page. If that’s not intentional, you could extend the dependency list accordingly.

Also worth ensuring resetPage from usePagination is useCallback‑stable; if its identity changes every render, this effect would fire on every render and continuously reset pagination.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d05bfb and 2226492.

📒 Files selected for processing (1)
  • app/markets/components/markets.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
app/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Next.js routes must live under app/ directory

Files:

  • app/markets/components/markets.tsx
**/*.{ts,tsx,js,jsx,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Run pnpm format (Prettier) before pushing with 2-space indentation, 100-character width, single quotes, and Tailwind-aware class ordering

Files:

  • app/markets/components/markets.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for React component file names (e.g., VaultBanner.tsx)

Files:

  • app/markets/components/markets.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{tsx,jsx}: Keep Tailwind class lists lean and dedupe variants with tailwind-merge
All toggles must use the shared IconSwitch component from @/components/common/IconSwitch for consistent sizing and animation

Files:

  • app/markets/components/markets.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

ESLint (Airbnb + Next.js) enforces hook safety and import hygiene

Files:

  • app/markets/components/markets.tsx
🧠 Learnings (2)
📚 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:

  • app/markets/components/markets.tsx
📚 Learning: 2024-11-25T09:39:42.148Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 87
File: app/home/HomePage.tsx:17-39
Timestamp: 2024-11-25T09:39:42.148Z
Learning: In `app/home/HomePage.tsx`, the `useEffect` hook depends on `[showCustomized]` because changing `showCustomized` triggers updates to the yield and risk terms.

Applied to files:

  • app/markets/components/markets.tsx

@antoncoding antoncoding merged commit 95a8cf3 into master Nov 30, 2025
4 checks passed
@antoncoding antoncoding deleted the fix/star-sort branch November 30, 2025 23:31
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.

bug: page change when stared

1 participant