feat: cluster disabled admin sidebar tabs at the bottom#9867
Open
feat: cluster disabled admin sidebar tabs at the bottom#9867
Conversation
Partition sidebar items into enabled and disabled groups. Enabled items render in their original sections, disabled items cluster at the bottom under their section headers. Remove <Disabled> wrapper in favor of SidebarTab's native disabled prop.
- Spending Limits (TOKEN_RATE_LIMITS) is now disabled when EE is off - Upgrade Plan moves to an unlabeled group at the very end of the list (only shown when there's no active subscription)
Contributor
|
Preview Deployment
|
Contributor
Greptile SummaryThis PR refactors the admin sidebar to cluster disabled items (Enterprise-gated features) at the bottom of the nav under their section headers, separated from enabled items by a divider. It also moves "Upgrade Plan" to an unlabeled group at the very bottom (only for non-subscribers), gates Token Rate Limits behind EE, and migrates Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildItems] --> B[allItems array]
B --> C[useFilter → filtered]
C --> D[enabled = filtered where !disabled]
C --> E[disabled = filtered where disabled]
D --> F[groupBySection → enabledGroups]
E --> G[groupBySection → disabledGroups]
F --> H{group.section?}
H -- No --> I[plain div wrapper]
H -- Yes --> J[SidebarSection]
K[disabledGroups.length > 0] --> L[Separator]
G --> M[SidebarSection with disabled prop]
M -- Bug: no section check --> N["SidebarSection title='' if UNLABELED"]
style N fill:#ffcccc,stroke:#ff0000
Prompt To Fix All With AIThis is a comment left during a code review.
Path: web/src/sections/sidebar/AdminSidebar.tsx
Line: 324-336
Comment:
**Missing section guard for UNLABELED disabled items**
The enabled-groups renderer at lines 299–320 checks `if (!group.section)` and wraps unlabeled items in a plain `div` to avoid rendering an empty section header. The disabled-groups renderer here is missing this guard — it unconditionally wraps every group in `SidebarSection` regardless of whether `group.section` is `""` (i.e. `SECTIONS.UNLABELED`).
There is a real code path that hits this today: `ADMIN_ROUTES.CUSTOM_ANALYTICS` is pushed with `SECTIONS.UNLABELED` and `disabled: !enableEnterprise` at lines 91–96. On any self-hosted instance without Enterprise, this item ends up in a `disabledGroup` with `section = ""`. Rendering `SidebarSection` with an empty `title` produces a sticky layout-consuming phantom header above the disabled tab.
The fix mirrors the pattern already used for enabled groups: add an `if (!group.section)` branch that returns a plain `div` (without a `SidebarSection` wrapper) for unlabeled disabled items.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat: gate Spending Limits behind EE, mo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+324
to
+336
| {disabledGroups.map((group, groupIndex) => ( | ||
| <SidebarSection | ||
| key={`disabled-${groupIndex}`} | ||
| title={group.section} | ||
| disabled | ||
| > | ||
| {group.items.map(({ link, icon, name }) => ( | ||
| <SidebarTab key={link} disabled icon={icon}> | ||
| {name} | ||
| </SidebarTab> | ||
| ))} | ||
| </SidebarSection> | ||
| ))} |
Contributor
There was a problem hiding this comment.
The enabled-groups renderer at lines 299–320 checks if (!group.section) and wraps unlabeled items in a plain div to avoid rendering an empty section header. The disabled-groups renderer here is missing this guard — it unconditionally wraps every group in SidebarSection regardless of whether group.section is "" (i.e. SECTIONS.UNLABELED).
There is a real code path that hits this today: ADMIN_ROUTES.CUSTOM_ANALYTICS is pushed with SECTIONS.UNLABELED and disabled: !enableEnterprise at lines 91–96. On any self-hosted instance without Enterprise, this item ends up in a disabledGroup with section = "". Rendering SidebarSection with an empty title produces a sticky layout-consuming phantom header above the disabled tab.
The fix mirrors the pattern already used for enabled groups: add an if (!group.section) branch that returns a plain div (without a SidebarSection wrapper) for unlabeled disabled items.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/sections/sidebar/AdminSidebar.tsx
Line: 324-336
Comment:
**Missing section guard for UNLABELED disabled items**
The enabled-groups renderer at lines 299–320 checks `if (!group.section)` and wraps unlabeled items in a plain `div` to avoid rendering an empty section header. The disabled-groups renderer here is missing this guard — it unconditionally wraps every group in `SidebarSection` regardless of whether `group.section` is `""` (i.e. `SECTIONS.UNLABELED`).
There is a real code path that hits this today: `ADMIN_ROUTES.CUSTOM_ANALYTICS` is pushed with `SECTIONS.UNLABELED` and `disabled: !enableEnterprise` at lines 91–96. On any self-hosted instance without Enterprise, this item ends up in a `disabledGroup` with `section = ""`. Rendering `SidebarSection` with an empty `title` produces a sticky layout-consuming phantom header above the disabled tab.
The fix mirrors the pattern already used for enabled groups: add an `if (!group.section)` branch that returns a plain `div` (without a `SidebarSection` wrapper) for unlabeled disabled items.
How can I resolve this? If you propose a fix, please make it concise.
jmelahman
approved these changes
Apr 2, 2026
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
- This PR looks safe to merge overall, with a minor UI-ordering issue rather than a functional break.
- In
web/src/sections/sidebar/AdminSidebar.tsx, "Upgrade Plan" is still handled as enabled, so when disabled tabs are present it may not stay pinned to the bottom as intended. - Given the moderate confidence and low-to-mild severity (4/10), the risk is mainly layout/UX consistency instead of regression in core behavior.
- Pay close attention to
web/src/sections/sidebar/AdminSidebar.tsx- ensure the "Upgrade Plan" entry is rendered after disabled items so it consistently appears at the bottom.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/src/sections/sidebar/AdminSidebar.tsx">
<violation number="1" location="web/src/sections/sidebar/AdminSidebar.tsx:167">
P2: "Upgrade Plan" is still treated as an enabled item, so it renders before the disabled cluster. When any disabled tabs exist, it won’t appear at the very bottom as intended. Consider rendering this entry after the disabled groups (or in its own trailing group) when there’s no subscription.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // 8. Upgrade Plan (admin only, no subscription) | ||
| if (!isCurator && !hasSubscription) { | ||
| items.push({ |
Contributor
There was a problem hiding this comment.
P2: "Upgrade Plan" is still treated as an enabled item, so it renders before the disabled cluster. When any disabled tabs exist, it won’t appear at the very bottom as intended. Consider rendering this entry after the disabled groups (or in its own trailing group) when there’s no subscription.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/src/sections/sidebar/AdminSidebar.tsx, line 167:
<comment>"Upgrade Plan" is still treated as an enabled item, so it renders before the disabled cluster. When any disabled tabs exist, it won’t appear at the very bottom as intended. Consider rendering this entry after the disabled groups (or in its own trailing group) when there’s no subscription.</comment>
<file context>
@@ -165,6 +162,16 @@ function buildItems(
+ // 8. Upgrade Plan (admin only, no subscription)
+ if (!isCurator && !hasSubscription) {
+ items.push({
+ section: SECTIONS.UNLABELED,
+ name: "Upgrade Plan",
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SidebarSectiongains adisabledprop (wraps title in<Disabled>) and usesHoverablefor action visibility<Disabled>wrapper — useSidebarTab's nativedisabledpropAddresses: https://linear.app/onyx-app/issue/ENG-3929/admin-sidebar-free-version-should-have-all-the-greyed-out-buttons-at.
Screenshots + Videos
Summary by cubic
Clusters disabled admin sidebar tabs at the bottom under their section headers, keeping enabled tabs in place and separated by a clear divider. Also gates Spending Limits behind EE and moves “Upgrade Plan” to an unlabeled bottom group when there’s no active subscription.
disabledprop toSidebarSectionand usesHoverablefrom@opal/corefor action visibility; removes the<Disabled>wrapper in favor ofSidebarTab’s nativedisabledprop from@opal/components.Written for commit 5f64583. Summary will update on new commits.