Repository navigation
0.7.0: appearance controls, community health files, and npm release setup - #8
Merged
Merged
Conversation
Appearance could only tune the dialog surface and backdrop: the title and body had no color or opacity of their own, the border had a color but no opacity, shadows were fixed presets with no way to aim them, and there was no hover styling at all. appearance now scopes color and opacity per component through surfaceColor, titleColor, titleOpacity, contentColor, contentOpacity, and borderOpacity. appearance.shadow additionally accepts a composed shadow, so both strength and direction are adjustable without hand-writing box-shadow: angle picks the direction the shadow falls, alongside distance, blur, spread, color, opacity, and inset. Presets and raw CSS values keep working unchanged. appearance.hover applies overrides only while the pointer is over the dialog, and only for dialogs that opt in, so every other dialog stays static. It reuses the color and shadow options and adds lift, scale, and duration. Hover rules set real properties rather than redefining the --fd-dialog-* custom properties, which are written inline and would win over any stylesheet rule on the same element. Hover transforms honour prefers-reduced-motion and are suspended while dragging, where a lift or scale would otherwise resize the box the drag logic clamps against and let a dragged dialog escape its bounds. The richer options ship as an opt-in capability behind forgedialog/appearance, matching dragging and animations. The main entry enables it automatically; forgedialog/core and the single-purpose entries keep a lightweight applier, which holds the alert-only fixture under its budget. Only the main entry's budget is raised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
cmm-cmm
force-pushed
the
claude/forgedialog-library-mhkpak
branch
from
September 1, 2026 14:07
65d742c to
a92724d
Compare
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The gzip budgets had almost no headroom left: index.cjs sat 0.09 KiB under its limit, the alert-only fixture 0.02 KiB, interactions 0.04 KiB, so ordinary changes tripped them. They are widened across the board, and the tarball budget with them. With that room, forgedialog/core now ships the full appearance applier, so per-component colors, composed shadows, and hover no longer need a separate import. The single-purpose entries keep the lightweight applier, and the alert-only fixture is unchanged at 5.48 KiB because those entries do not go through the core entry. Wiring core to the appearance capability surfaced that the registration never survived bundling. Capabilities register through a bare, side-effectful import, which a bundler keeps only when the resolved file is named in package.json sideEffects. ESM code splitting had moved the registerAppearanceApplier call into a shared chunk, and a chunk name cannot be listed there, so the import was discarded and forgedialog/appearance was silently a no-op. forgedialog/interactions and forgedialog/animations escaped it only through how their code happened to be chunked. Entries are now published unsplit, which costs duplication across entry files but makes every entry self-contained; consumers still only pay for the entry they import. check-capabilities.mjs bundles the published entries and asserts each capability registers while forgedialog/alert stays lightweight. It fails on the previous split output and passes on this one, and runs in validate and CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
Corner rounding was fixed to the global --fd-radius token, and the header had no background of its own, so neither could be set per dialog. appearance.radius takes one value for every corner, any CSS border-radius value, or a per-corner object whose omitted corners keep the theme radius. Drawers and bottom sheets now express their shapes through the radius variable rather than a hard-coded property, so they keep their defaults but follow an explicit radius when one is given. appearance.titleBackground paints the dialog header, which stays transparent by default and is clipped by the dialog's corners. A header with its own background is flagged so it reserves the bottom padding a bare header borrows from the body below it. Both options are available on hover too. Substituting the radius variable twice, as the bottom sheet did to square its lower corners, expanded a per-corner value into ten lengths and voided the declaration, leaving the sheet with no rounding at all; the bottom corners are squared with longhands instead. Covered by a browser test alongside the per-corner, plain sheet, and drawer shapes, since jsdom has no cascade to compute them from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
GitHub's community profile was missing everything except the README and license, so the repository had no stated contribution process, no conduct policy, and no private route for reporting a vulnerability. CONTRIBUTING.md documents the real workflow rather than a generic one: the validate gate CI runs, when API reports and changesets have to be refreshed, why capabilities register through side-effectful imports and what test:capabilities protects, that bundle budgets are enforced, and that computed-style behaviour belongs in e2e because jsdom has no cascade. SECURITY.md routes reports to a private advisory and scopes them. It names unsafeHtml as out of scope, since it is documented as trusted-input-only and sanitizes nothing by design, and points at html plus sanitizeHtml instead. CODE_OF_CONDUCT.md adopts Contributor Covenant 2.1. Issue forms ask which entry point was used, because the single-purpose entries ship the lightweight appearance applier and ignore the richer options. The pull request template mirrors the checks in CONTRIBUTING.md. Enforcement and security contacts point at GitHub's private reporting rather than an email address, so no personal address is published without asking. format:check now runs prettier over the whole repository instead of a hand-maintained path list, which is why these files were not covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
The release workflow published with a bare `npm publish --access public` and inherited whatever npm shipped with Node, which is the version trusted publishing depends on. It also had no guard against publishing the wrong version: a tag and package.json that disagree would have published whatever package.json happened to say. The workflow now installs a current npm explicitly and runs a release check first. check-release.mjs verifies the tag matches package.json, the package is not private, the metadata npm needs is present, and that version is not already on the registry, where versions are immutable and a duplicate would otherwise fail mid-release. A registry outage warns instead of failing, since npm refuses duplicates on its own. publishConfig.access is now public, so the flag is not needed at the call site, and CHANGELOG.md ships in the tarball so release notes travel with the package. docs/releasing.md covers cutting a release and the one-time setup this automation cannot do: the npm environment, the trusted publisher entry, and the NPM_PUBLISH_ENABLED variable that keeps the pipeline from publishing before someone means it to. npm documents trusted publishers on a package's settings page, which normally requires the package to exist, and forgedialog is unpublished. That could not be verified here because docs.npmjs.com is unreachable from this environment, so the document flags it and gives the token-based first publish as the fallback rather than asserting either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
The release job gates on the npm environment, but the setup guide only said to create it. It now spells out the configuration: a required reviewer, since a published version cannot be replaced, administrator bypass off so that gate is not decorative for the person most likely to release, and deployment rules for both the v* tag and main. Both rules matter. Releases run from a tag, but workflow_dispatch offers a dry run from main, and restricting the environment to tags alone would stop that run at the gate and fail the job instead of letting it validate and skip publishing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4
cmm-cmm
marked this pull request as ready for review
September 1, 2026 15:17
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 related pieces of work on the way to 0.7.0, in five commits.
1. Appearance: per-component color, corner radius, and hover
appearancepreviously reached only the dialog surface and backdrop. It now scopes styling per component, and adds two things that were not adjustable at all:surfaceColor,titleColor,titleBackground,titleOpacity,contentColor,contentOpacity,borderColor,borderOpacity. The title and body had no color of their own; the header had no background.radius— corner rounding was fixed to the global--fd-radiustoken. It now takes one value for every corner, any CSSborder-radiusvalue, or a per-corner object whose omitted corners keep the theme radius.shadow— still accepts a preset or a rawbox-shadow, and now the parts of one:anglepicks the direction the shadow falls (0up,90right,180down,270left), plusdistance,blur,spread,color,opacity,inset.hover— applied only while the pointer is over the dialog, and only for dialogs that opt in, so every other dialog stays static. Same color and shadow options pluslift,scale,duration. Honoursprefers-reduced-motion.Four bugs found by testing rather than by reading:
--fd-dialog-*custom properties. Those are written as inline styles byapplyAppearance, and an inline declaration beats a stylesheet rule on the same element, so the first implementation silently never applied hover colors.box-shadowandtransformworked, which is what exposed it.liftorscaleresizes the box the drag logic clamps against, letting a dragged dialog escape the viewport. Hover transforms are suspended while dragging. The existing drag bounds test caught this.0 0 0 0before,24px 24px 0 0after. Squared with longhands instead.import 'forgedialog/appearance'was silently a no-op. Capabilities register through a bare side-effectful import, which a bundler keeps only when the resolved file is named insideEffects. ESM code splitting had moved theregisterAppearanceAppliercall into a shared chunk, and a chunk name cannot be listed there.forgedialog/interactionsandforgedialog/animationsescaped this only through how their code happened to be chunked, not by design. Entries are now published unsplit.Budgets had almost no headroom left (
index.cjs0.09 KiB under, the alert-only fixture 0.02 KiB,interactions.cjs0.04 KiB), so ordinary changes tripped them. They are widened across the board. With that room,forgedialogandforgedialog/coreboth ship the full appearance applier, so nothing needs an extra import; the single-purpose entries keep the lightweight one and the alert-only fixture is unchanged at 5.48 KiB.2. Community health files
The community profile had only a README and a license. Added
CONTRIBUTING.md,SECURITY.md,CODE_OF_CONDUCT.md(Contributor Covenant 2.1), issue forms, and a pull request template.These document the repository's actual workflow rather than a generic one: the
validategate, when API reports and changesets must be refreshed, why capabilities register through side-effectful imports, that bundle budgets are enforced, and that computed-style behaviour belongs ine2ebecause jsdom has no cascade.SECURITY.mdscopes reports and namesunsafeHtmlas out of scope, since it is documented as trusted-input-only and sanitizes nothing by design.Contact points route to GitHub's private vulnerability reporting rather than an email address, so no personal address is published without the maintainer choosing to. Private vulnerability reporting must be enabled in Settings → Security for those links to work.
format:checknow runs Prettier over the whole repository instead of a hand-maintained path list — which is exactly why new root-level files were not covered by it.3. npm release pipeline
The release workflow published with a bare
npm publish --access publicand inherited whatever npm shipped with Node — the very thing trusted publishing depends on, and a version that varies by Node patch release. It also had no guard against a tag andpackage.jsondisagreeing.scripts/check-release.mjsverifies the tag matchespackage.json, the package is not private, required metadata is present, and that version is not already on the registry, where versions are immutable and a duplicate would otherwise fail mid-release. A registry outage warns rather than fails, since npm refuses duplicates on its own.publishConfig.accessispublic;CHANGELOG.mdnow ships in the tarball.docs/releasing.mdcovers cutting a release and the one-time setup this automation cannot do.Three things still need a human, all documented in
docs/releasing.md: create thenpmenvironment, add the trusted publisher on npmjs.com, and set theNPM_PUBLISH_ENABLEDrepository variable. Until that last one istrue, the release workflow runs every check and deliberately publishes nothing.One caveat stated honestly: npm configures trusted publishers from a package's settings page, which normally requires the package to exist, and
forgedialogis unpublished (registry returns 404). That could not be verified from primary sources here becausedocs.npmjs.comis blocked by this environment's egress proxy, so the document flags it and gives the token-based first publish as a fallback rather than asserting either way.Test plan
npm run validatepasses end to endnpm test— 172 tests pass, up from 162npm run test:capabilities— verified it actually catches the bundler bug: fails oncoreandappearancewith splitting re-enabled, passes with it offnpm run check:release— verified all three paths: no tag (pass), matching tag (pass), mismatched tag (fails with a clear message)npx playwright test --project=chromium— 15 passed, 1 skipped (mobile-only), against the pre-installed Chromiumangle, per-corner radius across modal/sheet/drawer shapes, and the header background reserving its paddingpublintclean,attwgreen across all four resolution modes, tarball 119.9 kB / 71 files90°and left at270°, dialog lifting 8px and scaling 1.02, top-left corner at 40px while the others stay 4px, header background clipped by the cornersThe visual regression baseline was refreshed once. That snapshot covers the whole demo page and the appearance builder gained controls, so the page got taller and the dialog shifted. Confirmed this was a fixture change and not a rendering regression: with the library changes but the original demo page,
alert visual remains stablepassed untouched — and CI's own browser then accepted the regenerated baseline.Note:
npm run test:e2ecannot run unmodified in the dev environment used here — the pinned@playwright/testwants browser build 1228 while only 1194 is installed, and downloads are disabled. The suite was run against the pre-installed Chromium via anexecutablePathoverride; CI installs its own browsers and passes.🤖 Generated with Claude Code
https://claude.ai/code/session_01MtJ3nDatGiv7dHjvZL1uZ4