fixes: Blank channel page edge case, prevents duplicate history entries on navigation#3410
fixes: Blank channel page edge case, prevents duplicate history entries on navigation#3410tzarebczan merged 1 commit intomasterfrom
Conversation
…es on navigation Ensures navigation actions triggered by buttons and tab changes do not result in duplicate entries in browser history. This prevents unnecessary back/forward navigation clutter. Loading indicators are adjusted to prevent flickering when switching between tabs or when sections are empty. Fixes an issue where empty playlist sections would not display the "No Content Found" message.
📝 WalkthroughWalkthroughThe changes add duplicate history entry prevention at multiple navigation points (button clicks, tab switches, and store-level) and refactor loading state logic to be more granular based on section type, with explicit empty-state messaging. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ui/component/button/view.jsx`:
- Around line 250-254: The click/navigation handler currently prevents
navigation if either path === currentPath or path === window.location.pathname,
which wrongly blocks cases where the current URL has a query string; remove the
second check (path === window.location.pathname) and only call
e.preventDefault() when path === currentPath (where currentPath is
window.location.pathname + window.location.search) so the component behavior
matches the store-level guard in ui/store.js; also update the inline comment to
state you are preventing duplicate history entries by comparing full
pathname+search.
| // Prevent duplicate history entries when navigating to the current page | ||
| const currentPath = window.location.pathname + window.location.search; | ||
| if (path === currentPath || path === window.location.pathname) { | ||
| e.preventDefault(); | ||
| } |
There was a problem hiding this comment.
Potential over-blocking: second condition may prevent legitimate navigation.
The condition path === window.location.pathname on Line 252 could incorrectly block navigation when the target path has no query string but the current URL does. For example:
- Current URL:
/channel?view=content - Target path:
/channel
These are different destinations, but the second condition would match and call preventDefault().
Consider removing the second condition to align with the store-level guard in ui/store.js, which only compares full paths (pathname + search):
Proposed fix
// Prevent duplicate history entries when navigating to the current page
const currentPath = window.location.pathname + window.location.search;
- if (path === currentPath || path === window.location.pathname) {
+ if (path === currentPath) {
e.preventDefault();
}📝 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.
| // Prevent duplicate history entries when navigating to the current page | |
| const currentPath = window.location.pathname + window.location.search; | |
| if (path === currentPath || path === window.location.pathname) { | |
| e.preventDefault(); | |
| } | |
| // Prevent duplicate history entries when navigating to the current page | |
| const currentPath = window.location.pathname + window.location.search; | |
| if (path === currentPath) { | |
| e.preventDefault(); | |
| } |
🤖 Prompt for AI Agents
In `@ui/component/button/view.jsx` around lines 250 - 254, The click/navigation
handler currently prevents navigation if either path === currentPath or path ===
window.location.pathname, which wrongly blocks cases where the current URL has a
query string; remove the second check (path === window.location.pathname) and
only call e.preventDefault() when path === currentPath (where currentPath is
window.location.pathname + window.location.search) so the component behavior
matches the store-level guard in ui/store.js; also update the inline comment to
state you are preventing duplicate history entries by comparing full
pathname+search.
1 participant
Fixes
Issue Number:
What is the current behavior?
What is the new behavior?
Other information
PR Checklist
Toggle...
What kind of change does this PR introduce?
Please check all that apply to this PR using "x":
Summary by CodeRabbit
Bug Fixes
UX Improvements