From 2c0d8d74e26a6d7b5864b2901c7227c2114680d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Manuel=20Fuentes=20Trejo?= Date: Mon, 27 Jul 2026 12:44:21 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix:=20let=20consumers=20ret?= =?UTF-8?q?heme=20Sidebar=20options=20through=20className?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NavigationOption hover/active colors lived in Sidebar.css keyed on `li[role='option']`. #677 dropped that invalid role from the `
  • `, so the rules stopped matching and every consumer theming through the `--konstruct-sidebar-hover-bg` / `--konstruct-sidebar-active-bg` custom properties silently fell back to the library default. Rather than re-point the selector, move the styling into the cva variants: a descriptor selector like `.konstruct-sidebar li:hover` (0,2,2) outranks a single utility class (0,1,0), so a consumer could never win through `className` even when the selector did match. - Drop the dead `li[role='option']` rules from Sidebar.css; the variants already cover the layout, padding and anchor styling they mirrored - Add the one rule that was not mirrored (`shrink-0`) to the variants - Prefix the active classes with `data-[active=true]:` so tailwind-merge detects the conflict against a consumer override and drops the default - Remove the now-unused `text-black` and duplicated `font-normal` - Cover both directions with tests: an override wins, no override keeps the default BREAKING CHANGE: `--konstruct-sidebar-hover-bg`, `--konstruct-sidebar-hover-color`, `--konstruct-sidebar-active-bg` and `--konstruct-sidebar-active-color` no longer do anything. Pass the colors to `NavigationOption` via `className` instead: `hover:bg-metal-800 data-[active=true]:bg-metal-800`. --- lib/components/Sidebar/Sidebar.css | 45 ------------------- lib/components/Sidebar/Sidebar.test.tsx | 37 +++++++++++++++ lib/components/Sidebar/Sidebar.types.ts | 6 +-- .../NavigationOption.variants.ts | 8 ++-- 4 files changed, 45 insertions(+), 51 deletions(-) diff --git a/lib/components/Sidebar/Sidebar.css b/lib/components/Sidebar/Sidebar.css index 2de657df..912327fc 100644 --- a/lib/components/Sidebar/Sidebar.css +++ b/lib/components/Sidebar/Sidebar.css @@ -64,51 +64,6 @@ aside.konstruct-sidebar[data-mode='collapsed'] { padding: 0 0.5rem; } -.konstruct-sidebar li[role='option'] { - flex-shrink: 0; -} - -/* - * Padding lives on the inner anchor (not the `
  • `) so the full - * hover/click area routes through the link — both navigation clicks and - * the Radix Tooltip trigger (in collapsed + expandOnHover mode) cover the - * whole option, not just the icon. - */ -.konstruct-sidebar li[role='option'] > a { - padding: 0.625rem; -} -.konstruct-sidebar[data-mode='expanded'] li[role='option'] > a { - padding: 0.625rem 1rem; -} - -/* - * Hover / active styling driven by CSS custom properties. The properties - * are inherited from `.konstruct-sidebar` (or a closer ancestor) so the - * consumer can theme with a single inline `style` on the wrapper, which - * outranks any un-layered Tailwind utility shipped by federated remote - * micro-frontends. - */ -.konstruct-sidebar li[role='option']:hover { - background-color: var(--konstruct-sidebar-hover-bg, #252a41); - color: var(--konstruct-sidebar-hover-color, #fff); -} -.konstruct-sidebar li[role='option'][data-active='true'] { - background-color: var(--konstruct-sidebar-active-bg, #252a41); - color: var(--konstruct-sidebar-active-color, #fff); -} - -.konstruct-sidebar li[role='option'] > a { - align-items: center; - color: inherit; - display: flex; - gap: 1rem; - text-decoration: none; - width: 100%; -} - -/* Center icon-only content (option icons and Logo) in collapsed mode. */ -.konstruct-sidebar[data-mode='collapsed'] li[role='option'], -.konstruct-sidebar[data-mode='collapsed'] li[role='option'] > a, .konstruct-sidebar[data-mode='collapsed'] [data-konstruct-sidebar-logo] > a { justify-content: center; } diff --git a/lib/components/Sidebar/Sidebar.test.tsx b/lib/components/Sidebar/Sidebar.test.tsx index 7273af5b..afa8955a 100644 --- a/lib/components/Sidebar/Sidebar.test.tsx +++ b/lib/components/Sidebar/Sidebar.test.tsx @@ -123,6 +123,43 @@ describe('Sidebar', () => { expect(mockOnClick).toHaveBeenCalledTimes(1); }); + it('should let a consumer className override the default active and hover colors', async () => { + const { getLink } = setup({ + options: ( + + Option 1 + + ), + }); + + const option = (await getLink(/option 1/i)).closest('li'); + + expect(option).toHaveClass('data-[active=true]:bg-metal-800'); + expect(option).toHaveClass('hover:bg-metal-800'); + expect(option).not.toHaveClass( + 'data-[active=true]:bg-kubefirst-dark-blue-800', + ); + expect(option).not.toHaveClass('hover:bg-kubefirst-dark-blue-800'); + }); + + it('should apply the default active color when no override is passed', async () => { + const { getLink } = setup({ + options: ( + + Option 1 + + ), + }); + + const option = (await getLink(/option 1/i)).closest('li'); + + expect(option).toHaveClass('data-[active=true]:bg-kubefirst-dark-blue-800'); + expect(option).toHaveClass('hover:bg-kubefirst-dark-blue-800'); + }); + it("shouldn't have accessibility violations", async () => { const { component } = setup({ options: ( diff --git a/lib/components/Sidebar/Sidebar.types.ts b/lib/components/Sidebar/Sidebar.types.ts index 38b56300..e64b5871 100644 --- a/lib/components/Sidebar/Sidebar.types.ts +++ b/lib/components/Sidebar/Sidebar.types.ts @@ -122,9 +122,9 @@ export interface Props * Inline styles applied to the wrapper `