-
-
Notifications
You must be signed in to change notification settings - Fork 51
fix(ui): move Sponsor/Help/Report Issue to sidebar footer (fixes #406) #443
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughTyped the sidebar’s data and permissions model, refactored nav filtering with useMemo, added footer actions (Sponsor, Help, Report Issue) with environment-aware issue URL. Removed sponsor/help/report from user menu. Simplified AppSidebar props by dropping toggleAddTeamModal. Updated selectors to typed AppState and constrained resources with a union type. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant SB as AppSidebar
participant ST as Store (AppState)
participant ACL as Permission Check
participant UI as Sidebar UI
U->>SB: Navigate / Render
SB->>ST: Select user, orgs, activeOrg
SB->>SB: useMemo reduce(data.navMain)
loop For each NavItem
SB->>ACL: hasAnyPermission(item.resource, subItems.resources)
ACL-->>SB: allow/deny per item/sub-item
end
SB->>UI: Render filtered nav + footer actions
sequenceDiagram
autonumber
participant U as User
participant SB as AppSidebar (Footer)
participant ENV as Client Env
participant GH as GitHub
U->>SB: Click "Report Issue"
SB->>ENV: Collect browser/OS/screen/lang/timezone/userAgent
SB->>SB: Build prefilled issue URL
SB->>GH: window.open(issue URL, _blank)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
view/components/layout/app-sidebar.tsx (1)
31-145
: Stop redefiningUser
/Organization
locally.The bespoke interfaces drop required fields like
username
andavatar
. As a resultuser
is now typed without those properties, so<NavUser user={user} />
fails type-checking (Property 'username' is missing…
) and the build breaks. Please reuse the canonical types coming from the Redux layer instead of shadowing them here—e.g. import the realUser
/Organization
(or at least theRootState
) and let the selectors infer their proper shapes.To unblock, delete the custom interfaces and change the selectors back to:
-const user = useAppSelector((state: AppState) => state.auth.user); -const organizations = useAppSelector((state: AppState) => state.user.organizations); -const activeOrg = useAppSelector((state: AppState) => state.user.activeOrganization); +const user = useAppSelector((state) => state.auth.user); +const organizations = useAppSelector((state) => state.user.organizations); +const activeOrg = useAppSelector((state) => state.user.activeOrganization);Then rely on the existing exported types for any further refinements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
view/components/layout/app-sidebar.tsx
(5 hunks)view/components/layout/nav-user.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
view/components/layout/app-sidebar.tsx (4)
view/lib/rbac.ts (2)
Resource
(4-20)useRBAC
(26-58)view/hooks/use-translation.ts (1)
useTranslation
(6-62)view/redux/hooks.ts (2)
useAppSelector
(5-5)useAppDispatch
(4-4)view/components/layout/nav-main.tsx (1)
NavMain
(32-87)
{t('user.menu.sponsor') || 'Sponsor'} | ||
</button> | ||
|
||
<button | ||
type="button" | ||
onClick={handleHelpClick} | ||
className="w-full py-2 rounded-lg border" | ||
> | ||
{t('user.menu.help') || 'Help'} | ||
</button> | ||
|
||
<button | ||
type="button" | ||
onClick={handleReportIssueClick} | ||
className="w-full py-2 rounded-lg border" | ||
> | ||
{t('user.menu.reportIssue') || 'Report Issue'} | ||
</button> |
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.
Fix the translation fallback.
t()
always returns a truthy string (the translated value or the key itself), so t('user.menu.sponsor') || 'Sponsor'
never hits the fallback and you end up rendering the raw i18n key while translations load. Compare against the key explicitly and reuse the helper for all three buttons:
+ const translateOrFallback = React.useCallback(
+ (key: string, fallback: string) => {
+ const value = t(key);
+ return value === key ? fallback : value;
+ },
+ [t]
+ );
...
- {t('user.menu.sponsor') || 'Sponsor'}
+ {translateOrFallback('user.menu.sponsor', 'Sponsor')}
...
- {t('user.menu.help') || 'Help'}
+ {translateOrFallback('user.menu.help', 'Help')}
...
- {t('user.menu.reportIssue') || 'Report Issue'}
+ {translateOrFallback('user.menu.reportIssue', 'Report Issue')}
📝 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.
{t('user.menu.sponsor') || 'Sponsor'} | |
</button> | |
<button | |
type="button" | |
onClick={handleHelpClick} | |
className="w-full py-2 rounded-lg border" | |
> | |
{t('user.menu.help') || 'Help'} | |
</button> | |
<button | |
type="button" | |
onClick={handleReportIssueClick} | |
className="w-full py-2 rounded-lg border" | |
> | |
{t('user.menu.reportIssue') || 'Report Issue'} | |
</button> | |
// (Some lines earlier in your component…) | |
const { t } = useTranslation(); | |
const translateOrFallback = React.useCallback( | |
(key: string, fallback: string) => { | |
const value = t(key); | |
return value === key ? fallback : value; | |
}, | |
[t] | |
); | |
// …then in your JSX render: | |
<button | |
type="button" | |
onClick={handleSponsorClick} | |
className="w-full py-2 rounded-lg border" | |
> | |
{translateOrFallback('user.menu.sponsor', 'Sponsor')} | |
</button> | |
<button | |
type="button" | |
onClick={handleHelpClick} | |
className="w-full py-2 rounded-lg border" | |
> | |
{translateOrFallback('user.menu.help', 'Help')} | |
</button> | |
<button | |
type="button" | |
onClick={handleReportIssueClick} | |
className="w-full py-2 rounded-lg border" | |
> | |
{translateOrFallback('user.menu.reportIssue', 'Report Issue')} | |
</button> |
🤖 Prompt for AI Agents
In view/components/layout/app-sidebar.tsx around lines 310 to 327, the code uses
t('user.menu.sponsor') || 'Sponsor' (and similar for Help and Report Issue) but
t() always returns a string so the fallback never runs; update the JSX to use a
reuseable check that compares the returned value to the key (e.g. if t(key) ===
key then use the human fallback) and apply that helper for all three buttons
(Sponsor, Help, Report Issue) so the readable fallback text is rendered while
translations are loading.
@kushu30 can you also attach a video recording for reference?, thanks! |
Summary
Moved Sponsor/Help/Report Issue to the sidebar footer and left Logout in profile dropdown.
Changes
How to test
Fixes #406
Summary by CodeRabbit
New Features
Refactor
UX Changes