Repository navigation
fix(ui): match the classic sidebar to its pre-migration look with core NavItem lg/solid - #35522
shah-harshit wants to merge 5 commits into
Conversation
Render left sidebar items at core's sm size (14px labels) so labels no longer read oversized or wrap, space rows to a 40px pitch, give the Settings/Logout footer the same row spacing, and make NavItemBase's collapsible-child indent size-aware so sm children line up with the parent label.
shah-harshit
added
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
Jest test CoverageUI tests summary
|
shah-harshit
changed the title
Add an lg size and a solid variant to core NavItemBase (28px icons, 56px rows, brand-filled current item and open-group header, child icons at lg) and use them in the classic LeftSidebar with the old 16px inset, 8px row gap and bordered open-group panel. Logout renders as a nav item so it matches Settings.
Drop the white bg-surface the core migration added to the classic aside and keep solid-variant NavItemBase rest items transparent, so the rail shows the #f8f9fc page background like the pre-migration sidebar.
The solid NavItemBase variant now renders resting icons and labels in one mid-gray (fg-tertiary/text-tertiary), as the pre-migration classic sidebar did, instead of core's lighter gray-400 icons and darker gray-700 labels.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| <details | ||
| className="tw:rounded-xl tw:open:border tw:open:border-t-0 tw:open:border-brand-subtle" | ||
| open={openKeys.includes(item.key)} | ||
| onToggle={(event) => toggleGroup(item.key, event.currentTarget.open)}> |
There was a problem hiding this comment.
💡 Quality: Open-group panel doesn't clip square child fills at its rounded corners
The open <details> panel has tw:rounded-xl and a 1px border-brand-subtle, but it has no overflow-hidden. At lg size, its children use tw:rounded-none (nav-item.tsx:63). When the last child of an open group is current (bg-brand-solid) or hovered (bg-brand-primary_alt), its square fill paints past the panel's curved bottom corners, so they look clipped against the border. Add tw:overflow-hidden to the <details> so child backgrounds follow its radius. The focus outline is offset, so check that it stays visible, or round the last child's bottom corners instead.
Clip child backgrounds to the panel's rounded corners.:
className="tw:overflow-hidden tw:rounded-xl tw:open:border tw:open:border-t-0 tw:open:border-brand-subtle"
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 closed / 1 findings🟡 Medium risk · New core sidebar sizing and solid styling, plus routing logout through a link control, could cause unexpected route or keyboard-activation behavior. Matches the classic sidebar to its pre-migration look using new core 💡 Quality: Open-group panel doesn't clip square child fills at its rounded corners📄 openmetadata-ui/src/main/resources/ui/src/components/MyData/LeftSidebar/LeftSidebar.component.tsx:238-241 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/app-navigation/base-components/nav-item.tsx:63 The open Clip child backgrounds to the panel's rounded corners.🤖 Prompt for agentsVerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
|
Combined into #35509; its commits are merged there and the description carries the sidebar details. Closing this one. |
Pull request was closed
2 participants
Describe your changes:
Fixes open-metadata/openmetadata-collate#7509
#35253 moved the classic left sidebar onto core
NavItemBase. Its defaultmdsize and light current-item style made the classic sidebar look different from the pre-migration one (sandbox): labels were 16px semibold, "Data Marketplace" wrapped, rows were packed tightly, and the current item and open groups lost their solid brand treatment. This PR makes the classic sidebar match sandbox using only core component APIs. AI mode uses a separate sidebar and is untouched.Core (
NavItemBase), additive. Existingsmandmdoutput is unchanged:size="lg"for roomy primary navigation: 56px rows (p-3.5), 28px icons, 14px/500 labels (600 when current),rounded-xl. Atlg, collapsible children keep their icon and sit flush (px-4 py-2.5, 48px rows) inside the open group.variant="solid":bg-primary);fg-tertiary/text-tertiary,#535862), as the old sidebar did, instead of the default's lighter gray-400 icons and darker gray-700 labels;bg-brand-solidand uses on-brand text and icon;bg-brand-solid_hover, with square bottom corners when open;collapsible-childindent is now per size (sm→pl-9,md→pl-10), so children line up with the parent label.Classic
LeftSidebar:tw:bg-surfacethat Migrate display, navigation, selection and numeric components to core #35253 added to the classic<aside>, so the rail shows the#f8f9fcpage background as before.size="lg" variant="solid", with the old layout:border-brand-subtlepanel with no top border.NavItemBase(a link-lessrole="link"), so it matches Settings. react-aria activates it on Enter. In the collapsed rail it stays an iconButton.Type of change:
High-level design:
N/A — small change.
Why core changes:
NavItemBasehas noclassNameprop, and its only sizes (sm,md) give 16–20px icons, 28–40px rows, a light current state, and white resting items. The pre-migration look (28px icons, 56px rows, a solid brand current item, transparent items) can't be reached from the caller without overriding core styles, which the design system forbids. So it is added as an opt-insize="lg"andvariant="solid". Both are additive: every existingsm/md/default usage renders identically. That includes the AI-mode shell, which doesn't useNavItemBase/NavListat all (verified below).Tests:
Use cases covered
Unit tests
nav-item.test.tsx(core):lg;solidcurrent item usesbg-brand-solid(defaultkeepsbg-brand-primary_alt), andsolidresting items are transparent withtext-tertiarylabels.LeftSidebar.test.tsx: Logout asks for confirmation on click and on Enter (9/9 passing).Backend integration tests
Playwright (UI) tests
landing-page,explore,ai-explore,glossary) are expected to change and need refreshing from CI renders.Manual testing performed
/my-data.UI screen recording:
Not applicable — styling fix. Screenshots below.
Measured (228px rail, Govern open)
#535862/#535862#a4a7ae/#414651#535862/#535862#f8f9fcpage bg / transparent#f8f9fcpage bg / transparent#1570efsolid#1570efsolidAI mode is unaffected
AI mode renders the
ai-shellsidebar, which imports neitherLeftSidebarnorNavItemBase/NavList. To check, I loaded both builds in AI mode at the same viewport and hashed the position, size, font, colour, and background of every sidebar element. Main and this PR are identical: 113 elements, hash-90309346.Collate (no Collate change needed)
Collate's classic mode renders this same OSS
LeftSidebar. It overrides only the item list (LeftSidebarClassCollate), and Collate ships no styles for.app-sidebaror the sidebar menu. So this PR fixes Collate's classic sidebar too, with no Collate change.To check, I ran Collate
origin/mainUI against this branch's OSS sources, and against OSSorigin/mainfor the "before" shots:#1570efcurrent item, and transparent items on the#f8f9fcpage background.ai-shellsidebar is identical before and after, at the same viewport: 176 elements, hash-1343543943.devrel still runs the pre-migration classic sidebar. Measured there: 14px/500 labels at x 67, 56px rows, 48px child rows, and a
#f8f9fcrail. That is within 1px of this PR (x 68). Its "Data Marketplace" row shows the hover tint, which is the same light-brand hover thesolidvariant applies.Checklist:
Fixesabove.🤖 Generated with Claude Code