fix: use push instead of default replace#2983
Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request updates several Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as Filter Component
participant H as useFilters Hook
participant Q as useQueryStates
participant B as Browser History
U->>F: Update filter settings
F->>H: Invoke useFilters with updated query parameters
H->>Q: Call useQueryStates(queryParamsPayload, { history: "push" })
Q->>B: Push new entry onto history stack
B-->>Q: Confirmation of history update
Q-->>H: Return updated query state
H-->>F: Render component with updated filters
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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. 🪧 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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (1)
29-31: Improved history navigation by using "push" instead of default "replace".This change aligns with best practices for managing browser history in an interactive filtering UI. By using
history: "push", each filter change will create a new browser history entry, allowing users to navigate backwards through their filtering steps with the browser's back button.Consider adding a debounce mechanism if filter changes can happen rapidly (e.g., during typing in a search field), as this could create too many history entries. For example:
const updateFilters = useCallback( (newFilters: RatelimitOverviewFilterValue[]) => { + // Use debounce if the filters can change rapidly const newParams: Partial<RatelimitQuerySearchParams> = { startTime: null, endTime: null, since: null, identifiers: null, }; // ... }, [setSearchParams], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/audit/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/logs/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
apps/dashboard/app/(app)/logs/hooks/use-filters.ts (5)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(17:24)apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(10:14)apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(30:38)apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(18:24)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(20:26)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (5)
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(18:24)apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(10:14)apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(30:38)apps/dashboard/app/(app)/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(22:31)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(17:24)
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (5)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(20:26)apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(10:14)apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(30:38)apps/dashboard/app/(app)/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(22:31)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(17:24)
apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (5)
apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(10:14)apps/dashboard/app/(app)/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(22:31)apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(18:24)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(20:26)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(17:24)
apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (5)
apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(30:38)apps/dashboard/app/(app)/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(22:31)apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (1) (1)
queryParamsPayload(18:24)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters.ts (1) (1)
queryParamsPayload(20:26)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1) (1)
queryParamsPayload(17:24)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: autofix
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
🔇 Additional comments (7)
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-filters.ts (1)
27-29: LGTM! Good UX improvement.Using
{ history: "push" }ensures that when filter parameters update, they create new history entries rather than replacing the current one. This allows users to navigate back through their filter history using the browser's back button.apps/dashboard/app/(app)/logs/hooks/use-filters.ts (1)
34-36: LGTM! Consistent with other filter hooks.This change modifies the browser history behavior to use "push" instead of the default "replace", which enhances UX by preserving filter history.
apps/dashboard/app/(app)/apis/_components/hooks/use-filters.ts (1)
17-19: LGTM! Good improvement to navigation.Using "push" for browser history allows users to use their browser's back button to navigate through filter changes, which is more intuitive than the default replacing behavior.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/hooks/use-filters.ts (1)
27-29: LGTM! Consistent history behavior.This change to use
{ history: "push" }matches the pattern applied to other filter hooks, ensuring consistent navigation behavior throughout the application.apps/dashboard/app/(app)/audit/hooks/use-filters.ts (1)
28-30: Good change: Improved navigation experience with history.pushChanging from the default history replacement to
history: "push"ensures that each filter operation creates a new browser history entry. This allows users to use the browser's back button to navigate through their previous filter states, greatly improving the navigation experience in the dashboard.apps/dashboard/app/(app)/apis/[apiId]/_overview/hooks/use-filters.ts (2)
41-43: Good change: Improved navigation experience with history.pushChanging from the default history replacement to
history: "push"ensures that each filter operation creates a new browser history entry. This allows users to use the browser's back button to navigate through their previous filter states, greatly improving the navigation experience in the dashboard.This change is consistently applied across multiple filter hook implementations, ensuring uniform behavior throughout the application.
1-196:Details
❓ Verification inconclusive
Verify browser back button behavior with multiple filters
The change to "push" history mode looks good, but it would be valuable to verify the user experience with multiple sequential filtering operations.
🌐 Web query:
What is the difference between history "push" vs "replace" in nuqs useQueryStates?💡 Result:
In nuqs'
useQueryStates, thehistoryoption determines how state changes affect the browser's history stack:Key Differences Between "push" and "replace"
Mode History Behavior Use Case Back Button Navigation pushCreates a new history entry for each state change Tracking navigational steps or state flow Users can backtrack through every change replaceUpdates the current history entry (default behavior) Local UI state adjustments Skips intermediate state changes Examples:
// Default replace behavior (squashes history) const [filters, setFilters] = useQueryStates( { category: parseAsString, sort: parseAsString }, { history: 'replace' } ); // Push mode for history traversal const [pagination, setPagination] = useQueryStates( { page: parseAsInteger, size: parseAsInteger }, { history: 'push' } );Implementation Details
- Replace mode acts like
git squash- merging all sequential updates into a single history entry[1][2][12]- Push mode maintains granular history entries but may require throttling (default: 50ms) to handle rapid updates[2][12]
- Options can be overridden per update:
setFilters({ category: 'books' }, { history: 'push' }); // Overrides hook-level configUser Experience Impact
- With
push, frequent updates (e.g., typing in search fields) create multiple history entries[6][7]replaceavoids "history pollution" but loses ability to backtrack through state changes[1][12]- Browser throttling applies to
pushto prevent performance issues (especially in Safari)[12]Choose based on whether state changes represent discrete navigation steps (
push) or temporary UI adjustments (replace)[12][7].Citations:
- 1: https://www.npmjs.com/package/next-usequerystate/v/1.12.1
- 2: https://www.npmjs.com/package/next-usequerystate
- 3: https://unpkg.com/browse/nuqs@1.19.3/dist/serializer-BZD8Ur_m.d.ts
- 4: https://nuqs.47ng.com/docs/batching
- 5: https://www.dhiwise.com/blog/design-converter/master-history-push-in-react-router-quick-guide
- 6: 47ng/nuqs#816
- 7: https://blog.logrocket.com/managing-search-parameters-next-js-nuqs/
- 8: https://dev.to/raaynaldo/react-router-usehistory-uselocation-and-useparams-10cd
- 9: https://unpkg.com/browse/nuqs@2.0.3/dist/_tsup-dts-rollup.d.ts
- 10: https://www.danielfullstack.com/article/this-library-could-replace-react-state-management
- 11: https://iq.js.org/questions/react/how-to-pass-params-to-historypush-method-in-react-router-v4
- 12: https://nuqs.47ng.com/docs/options
- 13: remix-run/react-router#9535
- 14: vercel/next.js#48110
- 15: vercel/next.js#18072
- 16: https://github.com/47ng/nuqs
- 17: https://v5.reactrouter.com/web/api/history
Action Required: Verify Browser Back Button Behavior with Multiple Filters
The update to use
"push"for the history mode inuseQueryStatesis appropriate given that each filter update now creates a separate history entry. Based on nuqs documentation, this means that every sequential filter change is tracked, allowing users to navigate back step-by-step. Please manually verify that with multiple sequential filtering operations, the browser's back button correctly reverts to the previous filter state(s) and that the accumulated history entries do not negatively impact user experience.
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit