From 97cb4395083a69afe8c34ad948134ac2d0872081 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Fri, 10 Jul 2026 13:10:46 -0700 Subject: [PATCH] site-builder: fix Menu guided panel (empty Colors) + surface layout/nav colors The shared NavStylePanel gated its Colors controls on the Navbar's prop names (backgroundColor/textColor/ctaColor), so a Menu -- whose color props are linkColor/linkHoverColor/ctaBgColor/ctaTextColor -- rendered an empty Colors section (customer report: 'nothing to select'). - Add navColorFields() helper: derives the visible color controls from the props actually present, covering both the Navbar and Menu schemas (+ Navbar's hoverColor, which render/toHtml consume but had no control). - Add a Menu Layout section (alignment/orientation/gap/font size), guarded on Menu's own props so it never leaks into Navbar or a standalone Logo. - Unit test (navColorFields.test.ts) locks each component to its real props. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/panels/right/styles/NavStylePanel.tsx | 77 +++++++++++++++++-- .../right/styles/navColorFields.test.ts | 55 +++++++++++++ craft/src/panels/right/styles/shared.tsx | 29 +++++++ 3 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 craft/src/panels/right/styles/navColorFields.test.ts diff --git a/craft/src/panels/right/styles/NavStylePanel.tsx b/craft/src/panels/right/styles/NavStylePanel.tsx index 995a3ed..1a60f32 100644 --- a/craft/src/panels/right/styles/NavStylePanel.tsx +++ b/craft/src/panels/right/styles/NavStylePanel.tsx @@ -9,6 +9,8 @@ import { PresetButtonGrid, CollapsibleSection, ColorPickerField, + navColorFields, + btnActiveStyle, labelStyle, inputStyle, smallInputStyle, @@ -56,6 +58,21 @@ export const NavStylePanel: React.FC = ({ selectedId, nodeProps /* Detect standalone Logo vs Navbar/Menu */ const isStandaloneLogo = nodeProps.type !== undefined && (nodeProps.type === 'text' || nodeProps.type === 'image') && nodeProps.logoText === undefined; + /* Color controls, derived from the props the selected component actually has */ + const colorFields = navColorFields(nodeProps); + + /* Menu layout controls (alignment/orientation/gap/font size). These props are + unique to the Menu component — Navbar uses navAlignment and has no + orientation/gap/fontSize — so guarding on their presence scopes this section + to the Menu without leaking into Navbar or a standalone Logo. */ + const hasMenuLayout = !isStandaloneLogo && ( + nodeProps.alignment !== undefined || + nodeProps.orientation !== undefined || + nodeProps.gap !== undefined || + nodeProps.fontSize !== undefined + ); + const GAP_PRESETS = ['8px', '16px', '24px', '32px', '40px'].map((g) => ({ label: g, value: g })); + return ( <> {/* Standalone Logo component settings */} @@ -165,17 +182,61 @@ export const NavStylePanel: React.FC = ({ selectedId, nodeProps )} - {/* Colors (not shown for standalone Logo - it has its own color picker) */} - {!isStandaloneLogo && ( + {/* Colors (not shown for standalone Logo - it has its own color picker). + Fields are derived from whichever color props the selected component + actually has, so Menu (linkColor/ctaBg/…) and Navbar (backgroundColor/…) + each get the right controls instead of an empty section. */} + {!isStandaloneLogo && colorFields.length > 0 && ( - {nodeProps.backgroundColor !== undefined && ( - setProp('backgroundColor', v)} /> + {colorFields.map((f) => ( + setProp(f.key, v)} + /> + ))} + + )} + + {/* Menu layout (alignment / orientation / gap / font size) */} + {hasMenuLayout && ( + + {nodeProps.alignment !== undefined && ( +
+ +
+ {(['left', 'center', 'right'] as const).map((a) => ( + + ))} +
+
)} - {nodeProps.textColor !== undefined && ( - setProp('textColor', v)} /> + {nodeProps.orientation !== undefined && ( +
+ +
+ {(['horizontal', 'vertical'] as const).map((o) => ( + + ))} +
+
)} - {nodeProps.ctaColor !== undefined && ( - setProp('ctaColor', v)} /> + {nodeProps.gap !== undefined && ( +
+ + setProp('gap', v)} /> +
+ )} + {nodeProps.fontSize !== undefined && ( +
+ + setProp('fontSize', e.target.value)} placeholder="14px" style={inputStyle} /> +
)} )} diff --git a/craft/src/panels/right/styles/navColorFields.test.ts b/craft/src/panels/right/styles/navColorFields.test.ts new file mode 100644 index 0000000..1c8767f --- /dev/null +++ b/craft/src/panels/right/styles/navColorFields.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest'; +import { navColorFields } from './shared'; +import { Menu } from '../../../components/basic/Menu'; +import { Navbar } from '../../../components/basic/Navbar'; + +/* + * Regression: the Menu component's color props (linkColor/linkHoverColor/ + * ctaBgColor/ctaTextColor) differ from the Navbar's (backgroundColor/textColor/ + * ctaColor). NavStylePanel's "Colors" section is shared across both, so it must + * derive its fields from the actual props present — otherwise a Menu shows an + * empty "Colors" dropdown with nothing to select. + */ +describe('navColorFields', () => { + it('surfaces the Menu color props (bug: was empty for Menu)', () => { + const menuProps = (Menu as any).craft.props; + const keys = navColorFields(menuProps).map((f) => f.key); + expect(keys).toContain('linkColor'); + expect(keys).toContain('linkHoverColor'); + expect(keys).toContain('ctaBgColor'); + expect(keys).toContain('ctaTextColor'); + }); + + it('never returns an empty list for a Menu (no empty Colors dropdown)', () => { + const menuProps = (Menu as any).craft.props; + expect(navColorFields(menuProps).length).toBeGreaterThan(0); + }); + + it('surfaces the Navbar color props (incl. hover/ctaText the panel never exposed before)', () => { + const navProps = (Navbar as any).craft.props; + const keys = navColorFields(navProps).map((f) => f.key); + // every field surfaced must be a real, consumed prop on the Navbar + expect(keys).toContain('backgroundColor'); + expect(keys).toContain('textColor'); + expect(keys).toContain('hoverColor'); + expect(keys).toContain('ctaColor'); + expect(keys).toContain('ctaTextColor'); + // must NOT invent Menu-only props on a Navbar + expect(keys).not.toContain('linkColor'); + expect(keys).not.toContain('ctaBgColor'); + }); + + it('does not surface Navbar-only props on a Menu', () => { + const menuProps = (Menu as any).craft.props; + const keys = navColorFields(menuProps).map((f) => f.key); + expect(keys).not.toContain('backgroundColor'); + expect(keys).not.toContain('hoverColor'); // Menu uses linkHoverColor + expect(keys).not.toContain('ctaColor'); + }); + + it('only returns fields whose prop is actually present', () => { + // A bare Logo (text logo) has no link/cta color props -> should not invent them + const keys = navColorFields({ backgroundColor: '#fff' }).map((f) => f.key); + expect(keys).toEqual(['backgroundColor']); + }); +}); diff --git a/craft/src/panels/right/styles/shared.tsx b/craft/src/panels/right/styles/shared.tsx index 60693f8..6a5fba4 100644 --- a/craft/src/panels/right/styles/shared.tsx +++ b/craft/src/panels/right/styles/shared.tsx @@ -247,6 +247,35 @@ export const ColorPickerField: React.FC = ({ label, value ); +/* ---------- Nav-family color fields ---------- + The Colors section is shared across the whole nav family (Navbar / Logo / + Footer / Menu). Those components DON'T share a color-prop schema: + Navbar/Logo/Footer use backgroundColor/textColor/ctaColor, while Menu uses + linkColor/linkHoverColor/ctaBgColor/ctaTextColor. Deriving the visible fields + from whichever props actually exist keeps the section from rendering empty + (the "Colors dropdown with nothing to select" bug on Menu). */ +export interface NavColorField { + key: string; + label: string; + fallback: string; +} +const NAV_COLOR_FIELDS: NavColorField[] = [ + // Navbar / Logo / Footer schema + { key: 'backgroundColor', label: 'Background', fallback: '#ffffff' }, + { key: 'textColor', label: 'Text Color', fallback: '#18181b' }, + { key: 'hoverColor', label: 'Hover Color', fallback: '#3b82f6' }, + { key: 'ctaColor', label: 'CTA Color', fallback: '#3b82f6' }, + // Menu schema (distinct prop names) + { key: 'linkColor', label: 'Link Color', fallback: '#3f3f46' }, + { key: 'linkHoverColor', label: 'Hover Color', fallback: '#3b82f6' }, + { key: 'ctaBgColor', label: 'CTA Background', fallback: '#3b82f6' }, + // ctaTextColor is shared: Menu's CTA text AND Navbar's CTA text + { key: 'ctaTextColor', label: 'CTA Text', fallback: '#ffffff' }, +]; +export function navColorFields(nodeProps: Record): NavColorField[] { + return NAV_COLOR_FIELDS.filter((f) => nodeProps[f.key] !== undefined); +} + /* ---------- Collapsible section ---------- */ export const CollapsibleSection: React.FC<{ title: string; defaultOpen?: boolean; children: React.ReactNode }> = ({ title, defaultOpen = true, children }) => { const [open, setOpen] = useState(defaultOpen);