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) <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,8 @@ import {
|
||||
PresetButtonGrid,
|
||||
CollapsibleSection,
|
||||
ColorPickerField,
|
||||
navColorFields,
|
||||
btnActiveStyle,
|
||||
labelStyle,
|
||||
inputStyle,
|
||||
smallInputStyle,
|
||||
@@ -56,6 +58,21 @@ export const NavStylePanel: React.FC<StylePanelProps> = ({ 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<StylePanelProps> = ({ selectedId, nodeProps
|
||||
</CollapsibleSection>
|
||||
)}
|
||||
|
||||
{/* 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 && (
|
||||
<CollapsibleSection title="Colors">
|
||||
{nodeProps.backgroundColor !== undefined && (
|
||||
<ColorPickerField label="Background" value={nodeProps.backgroundColor || '#ffffff'} onChange={(v) => setProp('backgroundColor', v)} />
|
||||
{colorFields.map((f) => (
|
||||
<ColorPickerField
|
||||
key={f.key}
|
||||
label={f.label}
|
||||
value={nodeProps[f.key] || f.fallback}
|
||||
onChange={(v) => setProp(f.key, v)}
|
||||
/>
|
||||
))}
|
||||
</CollapsibleSection>
|
||||
)}
|
||||
|
||||
{/* Menu layout (alignment / orientation / gap / font size) */}
|
||||
{hasMenuLayout && (
|
||||
<CollapsibleSection title="Layout">
|
||||
{nodeProps.alignment !== undefined && (
|
||||
<div style={sectionGap}>
|
||||
<label style={labelStyle}>Alignment</label>
|
||||
<div style={{ display: 'flex', gap: 4 }}>
|
||||
{(['left', 'center', 'right'] as const).map((a) => (
|
||||
<button key={a} onClick={() => setProp('alignment', a)} style={btnActiveStyle((nodeProps.alignment || 'right') === a)}>
|
||||
{a.charAt(0).toUpperCase() + a.slice(1)}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{nodeProps.textColor !== undefined && (
|
||||
<ColorPickerField label="Text Color" value={nodeProps.textColor || '#18181b'} onChange={(v) => setProp('textColor', v)} />
|
||||
{nodeProps.orientation !== undefined && (
|
||||
<div style={sectionGap}>
|
||||
<label style={labelStyle}>Orientation</label>
|
||||
<div style={{ display: 'flex', gap: 4 }}>
|
||||
{(['horizontal', 'vertical'] as const).map((o) => (
|
||||
<button key={o} onClick={() => setProp('orientation', o)} style={btnActiveStyle((nodeProps.orientation || 'horizontal') === o)}>
|
||||
{o.charAt(0).toUpperCase() + o.slice(1)}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{nodeProps.ctaColor !== undefined && (
|
||||
<ColorPickerField label="CTA Color" value={nodeProps.ctaColor || '#3b82f6'} onChange={(v) => setProp('ctaColor', v)} />
|
||||
{nodeProps.gap !== undefined && (
|
||||
<div style={sectionGap}>
|
||||
<label style={labelStyle}>Gap</label>
|
||||
<PresetButtonGrid presets={GAP_PRESETS} activeValue={nodeProps.gap} onSelect={(v) => setProp('gap', v)} />
|
||||
</div>
|
||||
)}
|
||||
{nodeProps.fontSize !== undefined && (
|
||||
<div style={sectionGap}>
|
||||
<label style={labelStyle}>Font Size</label>
|
||||
<input type="text" value={nodeProps.fontSize || '14px'} onChange={(e) => setProp('fontSize', e.target.value)} placeholder="14px" style={inputStyle} />
|
||||
</div>
|
||||
)}
|
||||
</CollapsibleSection>
|
||||
)}
|
||||
|
||||
@@ -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']);
|
||||
});
|
||||
});
|
||||
@@ -247,6 +247,35 @@ export const ColorPickerField: React.FC<ColorPickerFieldProps> = ({ label, value
|
||||
</div>
|
||||
);
|
||||
|
||||
/* ---------- 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<string, any>): 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);
|
||||
|
||||
Reference in New Issue
Block a user