Phase C: charts, dialogs and feedback specs — and the five defects they found (2.24.3) - #39
Merged
Merged
Conversation
1066 tests assert markup; none of them can see a colour. For a library whose product is its appearance that is the gap in the middle, and the popover spec already records it in its own words as 'no regression net'. Three findings on 2026-08-20 alone rested entirely on measurements taken by hand in a browser: the portal's discarded scroll, Block's 420/383/38, and I7 A's inverted loudness. Each was real, each is fixed, none is guarded. Draft, not Ready: three open points are the Product Owner's. The first is a CODE-03 sign-off that IDEA-05 says belongs in this dialogue rather than in a commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoXQ4L9XzgZJv5X9sZZb1L
The Product Owner pointed out that Playwright is already driving this UI daily, which is true and which the first draft did not account for. The distinction that survives: an MCP server lives in an agent's harness and leaves no trace in the repository, while a <PackageReference> binds every clone and every CI run. The distinction that does not: that CODE-03 reaches this at all. That rule is about what ships, and its check reads code/*/*.csproj, never tests/. What is left is a build decision rather than a sign-off, and it has a better answer than an exception: the suite becomes its own project outside DRYL.slnx with its own CI job, so 'dotnet test DRYL.slnx' stays self-contained and the CLAUDE.md stage 5 evidence bar is not touched. Recorded rather than quietly edited away — a withdrawn blocker is worth as much to a later reader as an approved one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoXQ4L9XzgZJv5X9sZZb1L
The four charts share two record types, two base classes and one palette. _Api.md now carries that contract - ChartSeries, ChartSegment, the two base classes, the ShowLegend and ValueFormat rules and the six-slot ceiling. _Interop.md records the category's zero-JS position and the one cleanup duty it does have (the AI aura's exit timer). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
F1 DrylLineChart, F2 DrylBarChart, F3 DrylAreaChart, F4 DrylDonutChart. Each carries its own criteria only; the shared contract stays in _Api.md. State: Implemented - the criteria were written from the code and the CSS, not from the doc comments, and 1066 tests stay green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
_Api.md carries DialogSize, DialogOptions, DialogParameters, DialogResult and the three interfaces the service and the provider meet over. _Interop.md carries dryl.modal, the exit-listener pair, the view-transition handoff and the provider's disposal duties - including why the exit watchdog is a rule and not a precaution. F1 DrylDialog is the frame, F2 DrylDialogProvider is the modality, F3/F4 are the two dialogs the service shows for ShowConfirmAsync/ShowAlertAsync. Written from the code, the CSS and the modal JS. Recorded gaps rather than glossed: no aria-describedby on the two built-ins, role=dialog where alertdialog fits, and the literal dialog widths. Also fixes a wrong rule citation in E4 F1 (UX-07 is about animation, not about decorative markup). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
_Api.md is mostly an inventory, because the eight components share almost nothing with each other - what they share is with the rest of the library. It records the one thing that is genuinely category-wide and genuinely awkward: three enums live at namespace level and five are nested, ProgressSize sits in a file named after another enum, and four separate size enums exist for what a reader would call one concept. All of it is bound by the 1.0 freeze, so it is recorded rather than corrected. _Interop.md is the unusual one. No component in the category obtains an IJSRuntime. DrylTooltip has an interop surface it never calls - one set of delegated document listeners drives every tooltip on the page, which is why a toolbar of triggers costs nothing and why a tooltip works during prerender. DrylNotifications gets its interop by composing DrylPopover. The cleanup that matters is the one subscription: a bell that does not unsubscribe from the scoped notification service is kept alive by it for the rest of the circuit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
F1 DrylAlert, F2 DrylTooltip, F3 DrylSpinner, F4 DrylSkeleton, F5 DrylProgress, F6 DrylEmptyState, F7 DrylErrorBoundary, F8 DrylNotifications. Written from the code, the CSS and the tooltip JS - not from the doc comments. Recorded gaps rather than glossed, and several of them are real: DrylProgress clamps its fill but not its ARIA value, so 120 of 100 shows a full bar and tells a screen reader "120"; the skeleton's base shimmer keeps running under prefers-reduced-motion because the calming rules only cover the AI mutations; DrylNotifications writes Read onto the consumer's own object in controlled mode; its unread dot's aria-label sits on a bare span and may never be announced; DrylAlert renders a dismiss button that does nothing when no handler is attached. Two AI-mode decisions are written down as decisions. DrylTooltip gets no Ai and no Aura: it renders no surface of its own, and its bubble is one page-level element shared by every tooltip. DrylNotifications takes Aura but no Ai, because the state belongs to each entry rather than to the inbox around them. State: Implemented for all eight - the criteria were written against the code, and 1066 tests stay green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
Four fixes in the library and one wrong sentence in the website catalog. The sixth item raised alongside them - an Ai parameter on DrylTooltip - is a new feature touching the AI visual vocabulary and goes to the idea stage instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
The fill was clamped into the track and aria-valuenow was not. Value=120 against Max=100 drew a full bar and told a screen reader "120 of 100"; a negative value drew an empty bar and reported the negative number. The half a sighted user cannot check was the wrong one. The fill width, the percentage label and the reported value now all come from one clamped number, so the drawn bar and the announced value cannot disagree. Max <= 0 is unchanged: the bar renders empty and reports zero. Seven tests, and they are the component's first of its own - it had two assertions in shared test files and nothing else. Spec F5 updated in the same commit (SPEC-01): four criteria under "Value and scale" and "Keyboard and accessibility", and the recorded gap that named this defect is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
DrylSkeleton's reduced-motion block calmed its AI states and dropped the stagger between its bars, and that read as the component honouring UX-06. It did not: the sweep lives on the shared .skel primitive in dryl.css, which no reduced-motion rule had ever touched. A user with the preference set got a placeholder that was entirely and permanently moving - and on a loading screen that is most of the page. The strip is no longer painted under prefers-reduced-motion: reduce. Measured in the browser in both modes: strip display none, and the blocks now rest on --glass-3 rather than --glass-1 (light 0.55 -> 0.72 alpha, dark 0.03 -> 0.08), which is the bright midpoint the sweep passed through - a still block is as legible as a moving one was at its clearest instead of resting at its dimmest. In light mode that is the difference between a placeholder and nothing. Streaming keeps its violet-cyan as a static tint on the block, so the signal the motion carried is not lost with the motion. Verified that with motion on everything is exactly as before: strip block, streaming background back on the strip. Fixed in the primitive, so DrylImage's loading state and any consumer of the skel classes are covered by the same change. UX-06's documented hit count moves 22 -> 23. Spec F4 updated in the same commit (SPEC-01); the recorded gap that named this defect is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
…d" out loud Two defects the F8 spec turned up, both in the same component. A controlled inbox set Read on the consumer's own DrylNotification and then raised OnMarkRead. Controlled mode's whole promise is the opposite - you own the state, the component raises callbacks - so a caller holding a snapshot found it changed underneath them, and the callback arrived describing something already done. The website's own controlled example had an empty OnMarkRead with a comment explaining that the component had already handled it: the demo documented the bug. Now the component raises the callback and writes nothing, and the example sets Read itself. The unread dot carried aria-label on a bare span. A generic element with no role is not reliably named, so a screen-reader user heard a row's title and time and nothing about it being unread - the one thing a notification bell exists to say. The word is now visually-hidden text inside the row's own button, and the dot is decorative so the state is announced once. Eight tests, the component's first. The controlled-mode one fails against the old behaviour, which is the point of it. Spec F8 updated in the same commit (SPEC-01); both recorded gaps are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
<DrylAlert Dismissible> on its own rendered a button that was focusable, announced, and completely inert: the alert never removed itself and nobody was listening, so pressing it did nothing. Which of the two parameters is set now decides who owns the alert's lifetime. With a handler nothing changes - dismissing stays a request the host answers. With none, the alert answers the press itself and animates out. Two options were rejected before this one. Hiding the button unless a handler is attached would silently drop a control the consumer asked for and make Dismissible mean two things. Always self-hiding would double up with hosts that unmount on OnDismiss and leave no way back. Self-hiding only when nobody is listening is one sentence and breaks nothing. DESIGN-12 makes the conditional unmount wrap in DrylPresence, and that wrapper would otherwise be a DOM change for every alert in every consumer app. It is rendered only in the self-dismissing configuration, so an alert with a handler and a non-dismissible alert are byte-identical to before; two tests hold that line. Measured in the browser: the handler-less alert goes presence-exit -> gone, and the alert beside it with a handler still sits directly in its parent. Nine tests, the component's first of its own. The removal completes on an animation end that bUnit cannot reach, so the suite asserts that it starts and the browser proved that it finishes. Spec F1 updated in the same commit (SPEC-01); the recorded gap that named this defect is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
The Product Owner asked for Ai and Aura on DrylTooltip, on the use case "an AI explains this element". The instinct is right - an explanation belongs next to the thing it explains - and the target is wrong. The bubble is one shared page-level element, which is what makes a toolbar of thirty tooltips free; it is aria-hidden, pointer-events:none, textContent-only, and it hides on pointerout. AI content put there is unreadable by a screen reader, unselectable, uncopyable, gone when the pointer moves, and fails WCAG 1.4.13. A bubble measured and clamped once cannot grow token by token either. So the document offers three options: provenance-only Ai on the tooltip; an explanation surface built on DrylPopover, which already portals, traps focus and animates out; or both, with the second carrying the weight. Impact is worked through per option against harness, specs, API and code. Three open points go back to the Product Owner. State stays Draft. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
Three answers, three open points closed. Ai and Aura go onto DrylTooltip; the explanation surface is not built now; the bubble carries a static provenance mark rather than the living aura, so the aura vocabulary stays in exactly one place and the JS-side reimplementation is refused. The AI-04-shaped question the mark raised - whether a reduced AI treatment is still one shared vocabulary - is signed off in the document rather than left as an assumption. Option B's shape is recorded even though it is deferred: its own component, not a mode of DrylPopover, so the decision is not re-litigated when it is raised. Open Points is empty. State stays Draft for one more beat: IDEA-06's last box is the Product Owner's explicit confirmation of the final version. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three bodies of work, stacked in the order they happened, because the later ones only exist because of the earlier ones.
1. Phase C: three more spec categories.
E4 Charts(4),E6 Dialogs(4) andE7 Feedback(8), each with its_Api.mdand_Interop.mdfilled in. Coverage moves 11/127 → 31/127. Every spec was reverse-engineered from the code, the CSS and the JS — deliberately not from the doc comments, which this repo has repeatedly found to be wrong.2. The five defects that reading produced. Writing a spec against the code rather than against its documentation is what turned these up. None of them was known before this branch:
DrylProgressannounced a value it did not draw. The fill was clamped into the track andaria-valuenowwas not, soValue="120" Max="100"drew a full bar and told a screen reader "120 of 100". The half a sighted user cannot check was the wrong one.DrylSkeleton's shimmer ignoredprefers-reduced-motion. The component's own reduced-motion rules calmed its AI states and dropped its stagger, which read as compliance. The sweep itself lives on the shared.skelprimitive indryl.css, which no reduced-motion rule had ever touched — so a user who asked for less motion got a placeholder that was entirely and permanently moving, on a screen that is mostly placeholder. Fixed in the primitive, soDrylImageis covered too.DrylNotificationswrote to the caller's own list in controlled mode, settingReadon the supplied object before raisingOnMarkRead— the opposite of what controlled mode promises. The website's own controlled example had an empty handler with a comment explaining that the component already did it: the demo documented the bug.DrylNotifications' unread state was never announced.aria-labelon a barespanwith no role is not reliably named, so a screen-reader user heard a row's title and time and nothing about it being unread — the one thing a bell exists to say.DrylAlert's dismiss button did nothing without a handler. Focusable, announced, inert.3. Two ideas.
I8(a regression net for what the library looks like) andI9(the AI tooltip — raised, challenged, narrowed to provenance-only on the maintainer's decisions,Open Pointsempty, stillDraftpending the final confirmation).Two calls worth reviewing rather than skimming
DrylAlertself-dismisses only when nobody is listening. Always self-hiding would double up with hosts that unmount onOnDismissand leave no way back; hiding the button unless a handler is attached would silently drop a control the consumer asked for.DESIGN-12requires the conditional unmount to wrap inDrylPresence, and that wrapper would otherwise be a DOM change for every alert in every consumer app — so it is rendered only in the self-dismissing configuration. Two tests hold that line.DrylNotifications' controlled fix is a visible behaviour change for anyone whoseOnMarkReadwas empty because the component did the work. Called out in bold in the changelog.Companion PR: Zimpi/DRYL.Website#11 (catalog text + two examples). The website consumes this repo by
ProjectReference, and its alert example shows the new self-dismissing path, so merge this one first.Type of change
Design-system checklist (see CLAUDE.md)
--glass-3on a resting skeleton block; the streaming tint is--ai-a/--ai-bviacolor-mixDESIGN-02)enumfor variants) — no new parameters in this PRDrylTooltip— unchangedAiState+.ai-aura*(no new AI states/colors)Docs (mandatory for library changes)
CHANGELOG.md—2.24.3cut with today's date;2.24.2was published, so a new block rather than an addition (REL-01,REL-02)<Version>bumped to2.24.3in the first library commit; the three later fixes join that block and leave it alone. PATCH carries the stack: no API is added or removedState: Implementedthroughout, and each recorded gap that named a fixed defect removed (SPEC-01,SPEC-04)ComponentCatalog— all eight Feedback components were already registered; the tooltip's stale description is fixed in the companion PR (REL-04)Verification
dotnet build DRYL.slnx -c Release— 0 errorsdotnet test DRYL.slnx -c Release— 1090 passed, up from 1066: 24 new tests in three new files (DrylProgressTests,DrylNotificationsTests,DrylAlertTests), the first of their own for all three componentsnode scripts/check-light-sync.mjsnode scripts/validate-light-contrast.mjsnode scripts/check-motion-tokens.mjsnode scripts/check-harness-links.mjsnode scripts/check-spec-coverage.mjs—31/127, still non-zero exit by design during phase CMeasured in the browser, not asserted:
display: none, blocks resting on--glass-3instead of--glass-1(light0.55 → 0.72alpha, dark0.03 → 0.08); with motion on, stripblockand the streaming gradient back on the strip — unchanged.presence presence--fade presence-exit→ gone. The alert beside it with a handler still sits directly in its parent (.col), so it gained no wrapper.One thing the test suite cannot do: the self-dismissal completes on an animation end that bUnit never fires, so the suite asserts that the alert begins to leave and the browser proved that it finishes. That boundary is written into the spec rather than papered over.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BoCEzbMKjESrSNDEZjcrnR