Add missing components to close the Figma component library gap - #99
Conversation
Adds a @surfnet/curve-contracts entry for every component in this batch (description-only where there's no variant/size/orientation axis), so both frameworks have a shared source of truth for names and docs before vendoring.
Vendors accordion, alert, alert-dialog, aspect-ratio, badge, button-group, calendar, carousel, chart, combobox, command, context-menu, date-picker, dialog, drawer, empty, hover-card, input-otp, item, kbd, native-select, navigation-menu, pagination, popover, progress, radio-group, resizable, scroll-area, sheet, skeleton, slider, sonner, spinner, switch, tabs, toggle, toggle-group, and tooltip via the shadcn CLI. date-picker is hand-composed from Calendar + Popover since shadcn has no registry item for it. Each component is wired to its @surfnet/curve-contracts entry, ships its own barrel and Storybook story, and is exported from the package entry. Also de-duplicates sibling files (button, input, textarea, input-group, separator, dialog, toggle) that the CLI vendored redundantly into new component folders instead of reusing the existing top-level ones, and promotes sidebar's internal sheet/skeleton/tooltip copies to the new top-level components.
Vendors accordion, alert, alert-dialog, aspect-ratio, badge, button-group, calendar, carousel, combobox, command, context-menu, date-picker, dialog, empty, hover-card, input-otp, item, kbd, native-select, navigation-menu, pagination, popover, progress, radio-group, resizable, scroll-area, slider, sonner, spinner, switch, tabs, toggle, and toggle-group via the Spartan CLI, plus stories for the already-vendored-but-unexported sheet, skeleton, and tooltip (previously internal Sidebar dependencies, now part of the public API). No Spartan equivalent exists yet for chart or drawer. Each component is wired to its @surfnet/curve-contracts entry and exported from public-api.ts. Replaces the Spartan CLI's default Lucide icons with Phosphor across the newly vendored files to match this package's existing icon convention, and drops the now-unused @ng-icons/lucide dependency.
Minor bump for @surfnet/curve-react and @surfnet/curve-angular.
Neither has a Spartan counterpart — chart has no generator at all, and Spartan's closest primitive to drawer is sheet, a different pattern already vendored as its own component. Keeping a React-only component breaks the in-parity contract this design system is built on (ADR-016); see ADR-019 for the follow-up plan.
Records why chart and drawer were removed (no Spartan equivalent) and tracks the components still fully out of scope from the original gap analysis — login, top navigation, and typography — so the follow-up work doesn't get lost.
…nt set The demo app's initial bundle now sits at ~1.08MB after this PR adds 36 components to @surfnet/curve-angular, tripping the CLI's 1MB hard budget in CI. Raise it to 1.5MB (warn at 1MB, was 500kB) to give headroom as the design system keeps growing, while still catching genuine bloat regressions.
…nents Restore the `'use client'` directive on the 33 newly vendored React component files that shadcn ships it on but the vendoring stripped. The library build preserves directives per module, so hook/context users (calendar, carousel, input-otp, sonner, ...) threw when imported into a Next.js server component. Close four contract-axis gaps where a real styled axis lived only in the component, not the shared @surfnet/curve-contracts spec: - context-menu item `variant` (ContextMenuItemVariantName) - alert-dialog content `size` (AlertDialogSizeName) - toggle-group `orientation` (ToggleGroupOrientationName) - sheet content `side` (SheetSideName) Each prop is now typed from the contract in both frameworks (sheet's Angular side stays Brain-delegated, matching slider/resizable), so a stray value fails the type-check instead of drifting silently.
| @@ -0,0 +1,7 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
|
|
|||
| export const kbdContract = defineContract({ | |||
There was a problem hiding this comment.
Suggestie: keyboard gewoon uitschrijven i.p.v. afkorten naar kdb in de bestandsnaam en contractnaam want dat is niet een algemeen bekende afkorting. Vooral ook toepassen in naam van storybook component en selector die consumers uiteindelijk afnemen. Of misschien beter KeyboardKey.
There was a problem hiding this comment.
lk wilde deze zo laten om een beetje in de buurt te blijven van shadcn (zoals elders beargumenteerd) maar ben er gisteren achtergekomen dat er een bestaand html element is <kbd> dat hetzelfde doet als dit (een beetje opmaak voor als iemand een beschikbaar keyboard shortcut visueel wil communiceren). Weer wat geleerd :)
| @@ -0,0 +1,7 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
|
|
|||
| export const inputOtpContract = defineContract({ | |||
There was a problem hiding this comment.
Suggestie: gebruik een volledig uitgeschreven term i.p.v. de afkorting otp om duidelijker te maken wat dit is. Belangrijkst is vooral hoe we de selector noemen die consumers afnemen en hoe de component heet in storybook.
Wel lastig om een duidelijke term te vinden die ook kort is. Het wordt typisch gebruikt voor 2FA maar zou ook werken voor andere toepassingen, dus de naam hoeft niet verplicht zo specifiek te zijn. Ideeën voor component-naam: InputCode, InputPassCode, InputSecurityCode, InputCharBoxes, ...
There was a problem hiding this comment.
Ik heb hiernaar gekeken, en mijn ervaring nu meenemend als newbie bij dit design system zou ik het denk ik voor de duidelijkheid hetzelfde laten als het component bij shadcn zodat de documentatie die daarbij voorhanden is nog van toepassing blijft. https://ui.shadcn.com/docs/components/base/input-otp
Ik begrijp je punt zeker, ook voor kbd, maar ik denk dat als we een vertaalslag introduceren met andere namen voor componenten dan de oorspronkelijke bron, dat het onderhoud een stukje lastiger blijft.
| export const Default: Story = { | ||
| render: (args) => ({ | ||
| props: args, | ||
| template: ` |
There was a problem hiding this comment.
Ik weet niet of het bewust is, maar je ziet altijd 6 boxes, ook als je met de interactieve control de lengte aanpast. Een kleiner instelbare lengte zorgt alleen dat hij de extra boxes leeg laat. Ik had verwacht dat het aantal boxes mee zou aanpassen.
There was a problem hiding this comment.
maxLength beperkt alleen het aantal tekens dat de onderliggende invoer accepteert; het creëert zelf geen zichtbare boxes. De voorbeelden in storybook zijn min of meer toevallig steeds met zes boxes, maar zouden net zo goed met minder kunnen zijn (en daar kunnen we een story voor toevoegen).
| description: 'Whether one or multiple items can be expanded at the same time.', | ||
| table: { defaultValue: { summary: 'single' } }, | ||
| }, | ||
| orientation: { |
There was a problem hiding this comment.
In storybook lijkt de orientatie niks te doen, en de story 'Horizontal' lijkt ook precies hetzelfde als de default.
There was a problem hiding this comment.
Blijkbaar is orientation in Shadcn deprecated, het enige verschil dat het maakt is welke pijltjes je gebruikt om naar het volgende element te gaan (links-rechts, en boven-beneden). Aangezien dat voor mij extra functionaliteit is en niet persé heel relevant heb ik om de verwarring minder te maken de orientation control weggelaten uit de story.
| type Story = StoryObj<HlmCarousel>; | ||
|
|
||
| /** Interactive playground — flip `orientation` in the controls to see both directions. */ | ||
| export const Default: Story = { |
There was a problem hiding this comment.
De knoppen van de carousel staan vlak naast elkaar in Angular, in plaats van links/rechts of boven/onder de items. In React ziet het er een stuk beter uit.
There was a problem hiding this comment.
In mijn lokale versie van deze branch zien ze er allebei hetzelfde uit. Ik denk (met sommige reacties van je) dat je een versie zag die niet helemaal goed gerebuild was of zo. Natuurlijk kan het ook aan mij liggen, maar ik neem voor nu even aan van niet :)
| }; | ||
|
|
||
| /** Checkbox items for toggling options. */ | ||
| export const WithCheckboxItems: Story = { |
There was a problem hiding this comment.
WithCheckboxItems en WithRadioItems toont een foutmelding als je rechts klikt, alleen in React storybook.
| type Story = StoryObj<typeof meta>; | ||
|
|
||
| /** A single-date picker — click the trigger to open the calendar popover. */ | ||
| export const Default: Story = { |
There was a problem hiding this comment.
Storybook toont 4 controls die allemaal niet instelbaar zijn.
There was a problem hiding this comment.
De DatePicker in Angular heeft 3 controls, die in React heeft er 4. Beide implementaties verschillen erg van elkaar. Ik maak hier een aparte issue voor aan
There was a problem hiding this comment.
Ik heb nog eens door de applicatie heengekeken en dat de implementaties functioneel verschillen is volgens de eerdere ideeen over het DS niet erg; dus ik laat dat zo.
| * `showCloseButton={false}` on `DialogContent` hides the corner close icon; pair it with | ||
| * `DialogFooter`'s own `showCloseButton` to give the user a single, explicit way to dismiss. | ||
| */ | ||
| export const WithoutCloseButton: Story = { |
There was a problem hiding this comment.
Deze toont toch wel een close button. Bij Angular werkt het wel zoals verwacht.
There was a problem hiding this comment.
Ik zie in React geen close button, bij mij werkt dit dus als verwacht
| * which collides with Storybook's own `render` story field when spread through | ||
| * `{...args}`, so this story hardcodes its props instead. | ||
| */ | ||
| export const Default: Story = { |
There was a problem hiding this comment.
Hier staan drie instelbare parameters in storybook die je niet kunt instellen.
There was a problem hiding this comment.
The default 6-digit OTP input. maxLength/disabled aren't exposed as live controls — the underlying input-otp library also defines its own render prop, which collides with Storybook's own render story field when spread through {...args}, so this story hardcodes its props instead.
Ik vind de documentatie momenteel ook niet duidelijk genoeg! Ik snap waarom je denkt dat je die parameters niet kunt instellen, maar volgens mij is het idee achter de controls vooral dat je kunt spelen met varianten die visueel iets wijzigen en functioneel bij uitzondering. Maar is het de bedoeling dat in de implementatie in de losse projecten de developers de functionaliteit verder uitbouwen zodat het inhaakt op de rest van hun systeem. Dus in het design systeem doen we niets met hoe data wordt opgeslagen of verzonden, maar ik verwacht wel dat we iets moeten toevoegen over hoe foutmeldingen bij formulieren getoond worden.
| @@ -0,0 +1,19 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
There was a problem hiding this comment.
Er is niet veel verschil tussen de gewone Select en NativeSelect en dan heb je daarnaast ook nog de Combobox, die heel anders heet maar eigenlijk een geavanceerdere select is of een multiselect. Als je dit allemaal ontsluit dan vraagt dat om verwarring en/of inconsistentie bij de afnemers.
Wellicht een idee voor op de backlog dat Curve de beschikbare opties enigszins inperkt en op versimpelde manier aanbiedt. Bijvoorbeeld één Select met een parameter of je input kan filteren of niet, en één MultiSelect. Het kan onderdeel zijn van de vervolgstory om te bepalen wat Curve wil aanbieden en waar je de splitsing wilt tussen aparte componenten of parametrisering.
There was a problem hiding this comment.
Hier ben ik het helemaal mee eens, en mijn voorkeur zou dan gaan naar zo dicht mogelijk bij de native <select>, dus NativeSelect
There was a problem hiding this comment.
Ik heb hier ook een aparte issue voor gemaakt
| }; | ||
|
|
||
| /** The `align` prop controls where the popup is anchored relative to its trigger. */ | ||
| export const Alignment: Story = { |
There was a problem hiding this comment.
Deze story toont alignment-opties en dat is niet aanwezig bij Angular. Bij Angular heb je weer een Vertical story die ik hier niet zie. Verschil in functionaliteit, en willen we die opties dan niet weghalen als ze niet door beide frameworks ondersteund worden?
Daarnaast wordt de title en description van elke nav-optie naast elkaar getoond bij de meeste story's van deze component. Bij Angular ziet het er netter uit omdat ze daar steeds boven elkaar staan. Verder heeft React pijltjes op de nav-buttons en Angular niet.
There was a problem hiding this comment.
Ik heb de navigation-menu versie die voor Angular is ontwikkeld vanuit Brain geupdate, en de stylingverschillen die me opvielen bij de verticale variant gelijkgetrokken.
| * The composed numbered-pagination block: page links, previous/next controls, and a | ||
| * page-size selector, driven by two-way `currentPage` / `itemsPerPage` bindings. | ||
| */ | ||
| export const NumberedPagination: Story = { |
There was a problem hiding this comment.
Deze story toont opties die ik in React niet terug zie. Bij React is er in plaats van deze een Interactive story.
There was a problem hiding this comment.
Ik heb dit gelijkgetrokken!
| }; | ||
|
|
||
| /** The popup can open on any side of the trigger, flipping to stay in view. */ | ||
| export const Placement: Story = { |
There was a problem hiding this comment.
Deze story/functionaliteit ontbreekt in Angular. Willen we dit dan wel ontsluiten voor React?
Functioneel zie ik weinig verschil met de Dialog component. Is het alleen dat deze door de knop getriggerd wordt en naast de knop verschijnt, terwijl de dialog algemeen midden in het scherm komt? Ik zou verwachten dat deze component alleen de trigger en locatie hoeft te regelen en dat de inhoud van de popup een dialog is of een gedeelde dialog-content o.i.d.
There was a problem hiding this comment.
De oorsprong van popover, is denk ik het html attribuut popover wat niet lang geleden is toegevoegd aan de spec. Die is verschillend van de Dialog component.
Dit is een blog over het verschil van Hidde de Vries: https://hidde.blog/dialog-modal-popover-differences/
(enigzins een deep dive :) ). Samengevat lijken ze op elkaar, maar een dialog kan interactie met de rest van de pagina blokkeren, compleet met focus trap. Een popover doet dit nooit. Ik zou die logica aan houden en dan beide aanbieden.
| * `showCloseButton={false}` on `SheetContent` hides the corner close icon — pair it with an | ||
| * explicit `SheetClose` action in the footer so the user still has a way to dismiss. | ||
| */ | ||
| export const WithoutCloseButton: Story = { |
There was a problem hiding this comment.
Zowel bij Angular als React is de close-button er nog gewoon en werkt die ook.
There was a problem hiding this comment.
Met "WithoutCloseButton" wordt bedoeld het kruisje rechtsboven, de sluit knop onderin blijft altijd bestaan volgens deze implementatie
There was a problem hiding this comment.
Wanneer ik beide storybooks inlaad, werkt dit zoals verwacht en zie ik het kruisje rechtsboven niet bij de "WithoutCloseButton" variant
| @@ -0,0 +1,7 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
|
|
|||
| export const sonnerContract = defineContract({ | |||
There was a problem hiding this comment.
Ik heb nog nooit van een sonner gehoord, zullen we het component gewoon Toast noemen of iets nog explicieter zoals Notification, BriefNotification, ToastMessage o.i.d.?
There was a problem hiding this comment.
Dat lijkt mij ook een beter idee, maar ik zie dat het bij Shadcn een apart component is van het toast component. Na mijn vorige reactie over het gelijk houden van benamingen zouden we daarom dit component deze naam moeten laten houden. Lastige!
| @@ -0,0 +1,19 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
|
|
|||
| export const switchContract = defineContract({ | |||
There was a problem hiding this comment.
Nog een backlog-idee voor naamgeving: een toggle en switch klinkt voor mij als bijna hetzelfde en je kunt ze ook interchangeably gebruiken. Bij een kort woord of symbool wordt het vaak een button en bij een form field of langere tekst zet je een switch naast de tekst. Misschien kan de naamgeving van de componenten worden aangepast naar ToggleField/ToggleInput, ToggleButton en ToggleButtonGroup o.i.d. zodat afnemers beter zien wat de vergelijkbare opties zijn.
There was a problem hiding this comment.
Dit, en je andere ideeen voor betere benamingen, spaar ik even op om er een issue van te maken!
| @@ -0,0 +1,8 @@ | |||
| import { defineContract } from './define-contract.js'; | |||
|
|
|||
| export const tooltipContract = defineContract({ | |||
There was a problem hiding this comment.
Ik zie veel overlap in functionaliteit tussen de HoverCard en Tooltip, de tooltip is eigenlijk een HoverCard met alleen (rich) tekst. Dus puntje voor de backlog om vanuit Curve te besluiten of dit niet als één gecombineerde component ontsloten moet worden.
Daarnaast zie ik ook heel veel overlap in verschillende vormen van popups. Buiten de logica van hoe het triggert (click of hover), locatie van een popup (bij element of centraal) en of het de achtergrond wel of niet blurred, is de popup zelf eigenlijk gewoon een Card die hergebruikt zou kunnen worden in ContextMenu, DropdownMenu, NavigationMenu, PopOver, HoverCard en Tooltip (en misschien zelfs in Combobox en Select). Dat zou ook veel kunnen helpen om consistente layout te krijgen (paddings, margins etc) tussen verschillende vormen van popups.
There was a problem hiding this comment.
Ik heb hier een issue voor gemaakt zodat het de aandacht krijgt die het verdient!
3b28b2b to
8d87b19
Compare
Summary
Closes the gap identified against the Figma component library (16 present / 41 missing) by adding components to both
@surfnet/curve-reactand@surfnet/curve-angular.chartanddrawer— tracked in ADR-019 (docs/decision-log.md) rather than shipped React-only, so we don't quietly break framework paritysheet,skeleton, andtooltipexisted internally in@surfnet/curve-angular(asSidebardependencies) but weren't exported — they're now part of the public APIEvery component has a
@surfnet/curve-contractsentry (description-only, or with a variant/size/orientation axis enforced viasatisfies Record<...>on both frameworks) and a Storybook story covering its full surface. Along the way, also de-duplicated sibling files the shadcn CLI vendors redundantly into new component folders (now reusing the existing top-levelbutton/input/separator/etc.), and replaced Spartan's default Lucide icons with Phosphor across the newly vendored Angular files to match this repo's existing icon convention.A couple of framework-surface gaps are documented in code rather than papered over: Angular's
scroll-area(wrapsngx-scrollbar, which auto-detects orientation) has no matchingorientationcontrol, and Angular's vendorednavigation-menuis missingalign/navOffsetinputs that upstream Spartan docs now expose.Test plan
pnpm --filter @surfnet/curve-contracts lint/buildpnpm --filter @surfnet/curve-react lint/buildpnpm --filter @surfnet/curve-angular lint/buildpnpm formatbuild-storybookfor both packages