-
Notifications
You must be signed in to change notification settings - Fork 25
fix: Navigate to the view tab on config page view #2802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ew tab highlighting by matching activeTabName to the view ID.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
❌ Deploy Preview for flanksource-demo-stable failed. Why did it fail? →
|
WalkthroughThe changes refactor the Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
❌ Deploy Preview for clerk-saas-ui failed. Why did it fail? →
|
There was a problem hiding this 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)
src/pages/config/details/ConfigDetailsIndexRedirect.tsx (1)
27-29: Guard on config loading/error states in the effect.The effect currently only guards on view loading/error states, but should also check
isConfigLoadingandisConfigErrorto prevent executing redirect logic when config data isn't ready.🔎 Proposed fix
useEffect(() => { - if (isLoading || isViewsError) { + if (isConfigLoading || isConfigError || isLoading || isViewsError) { return; } if (!id || tabs.length === 0) { return; } const firstTab = tabs[0]; if (!firstTab || location.pathname === firstTab.path) { return; } navigate( { pathname: firstTab.path, search: firstTab.search ?? location.search }, { replace: true } ); }, [ + isConfigLoading, + isConfigError, isLoading, isViewsError, id, tabs, navigate, location.pathname, location.search ]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Configs/ConfigDetailsTabs.tsxsrc/components/Configs/ConfigTabsLinks.tsxsrc/pages/config/details/ConfigDetailsIndexRedirect.tsxsrc/pages/config/details/ConfigDetailsViewPage.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/Configs/ConfigDetailsTabs.tsx (1)
src/components/Configs/ConfigTabsLinks.tsx (1)
useConfigDetailsTabs(17-119)
src/components/Configs/ConfigTabsLinks.tsx (1)
src/api/types/configs.ts (1)
ConfigItem(49-91)
src/pages/config/details/ConfigDetailsIndexRedirect.tsx (1)
src/components/Configs/ConfigTabsLinks.tsx (1)
useConfigDetailsTabs(17-119)
🔇 Additional comments (8)
src/pages/config/details/ConfigDetailsViewPage.tsx (1)
32-32: LGTM: Correct fix for tab highlighting.This change correctly aligns the
activeTabNamewith the tab'skeyproperty (which is set toview.idinuseConfigDetailsTabs), ensuring the active tab is properly highlighted when viewing a config view page.src/components/Configs/ConfigDetailsTabs.tsx (1)
48-48: LGTM: Correctly updated to match the new hook API.The destructuring properly extracts the
tabsarray from the new object-based return value ofuseConfigDetailsTabs.src/pages/config/details/ConfigDetailsIndexRedirect.tsx (3)
20-24: LGTM: Correctly extracts loading and error states.The destructuring properly handles the new object-based return value from
useConfigDetailsTabs, enabling proper coordination between config and view loading states.
58-64: LGTM: Correctly merges loading states.Properly handles loading states from both config and view queries, ensuring the loading indicator displays until both are ready.
66-72: LGTM: Correctly merges error states.Properly handles error states from both config and view queries with an appropriate error message.
src/components/Configs/ConfigTabsLinks.tsx (3)
17-21: LGTM: Well-structured return type.The new return type properly exposes loading and error states alongside the tabs array, enabling better coordination and user experience across consuming components.
24-32: LGTM: Correctly extracts query states.Properly destructures the loading and error states from the
useQueryhook for propagation to consumers.
114-118: LGTM: Consistent return structure.Both return paths correctly wrap the tabs array in the new object structure along with loading and error states.
Summary by CodeRabbit
Release Notes
Bug Fixes
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.