Skip to content

docs: address #3236 review feedback (Cosmos default, stylesheet setup, CHANGELOG/ROADMAP corrections) - #3240

Open
showerbee wants to merge 3 commits into
masterfrom
docs/readme-review-followups
Open

showerbee wants to merge 3 commits into
masterfrom
docs/readme-review-followups

Conversation

@showerbee

Copy link
Copy Markdown
Collaborator

Follow-up to #3236

Additional description

Docs-only follow-up to #3236, which merged before its review feedback was addressed. It covers @interactivellama's review comment and the findings from a second review pass. Each finding was checked against git history and the published tarballs.

README (docs(readme))

  • Makes Cosmos the default theme, per @interactivellama's comment. The Highlights section and the stylesheet table now list Cosmos first.
  • Fixes the Styling example. The old <link href="/node_modules/..."> only worked in Storybook, which maps dist/css to /slds2 in .storybook/main.ts. Installing the package doesn't serve node_modules over HTTP, so anyone copying that example got unstyled components. The section now shows a bundler CSS import first, then a serve-and-link alternative for setups without a bundler.
  • Adds a note that the SLDS 2 stylesheet references images through ../../public/ URLs, and that @salesforce-ux/design-system-2 2.264.x doesn't ship them. Vite warns and builds anyway. webpack's css-loader fails unless its url option skips those paths. This gap is upstream, in the SLDS 2 package, and is worth raising with the SLDS team.

CHANGELOG (docs(changelog))

  • alpha.3: alpha.2 already published with --tag alpha in the workflow. Chore/update package to use alpha tag #3220 made alpha the package default through publishConfig.tag.
  • alpha.2: says components were migrated to TypeScript and many rewritten as function components. Some class components remain, for example AlertContainer and ToastContainer.
  • alpha.2 and alpha.3 Removed: lists the irregular SLDSMenuPicklist alias and the SLDS-prefixed alias of each removed component. These were checked against the root exports of the 0.10.65 and alpha tarballs.

ROADMAP (docs(roadmap))

  • forms has a nested story and test (forms/input/__docs__/inline, forms/input/__tests__/inline.test.jsx), so it's removed from the no-story and no-test lists.
  • Story snapshot coverage is qualified. The snapshot glob __docs__/*.stories.* skips nested stories, so InlineInput and SearchInput are listed as a P2 gap.
  • The popper-shape adapter is attributed correctly: it's computeFloatingData in utilities/dialog/index.jsx, and dialog-helpers.js only consumes its output.
  • Removes "non-coverable content" from the VisualPicker gaps, because the coverable prop already exists.
  • "No Storybook deployed" is now "no public v11 documentation site". Chromatic does build Storybook on push.

Not in this PR: @interactivellama also asked for Cosmos to be the default theme in Storybook. That one is for @Niznikr, so this PR doesn't touch .storybook/.

Next: a fix: PR to loosen the @salesforce-ux/design-system peer dependency, which is pinned to 2.25.0-alpha.2. That PR will also simplify the README's ERESOLVE troubleshooting.


CONTRIBUTOR checklist (do not remove)

Please complete for every pull request.

Green gate — all of these must pass locally (CI runs the same):

  • npm run lint passes with 0 errors (npm run lint:fix to auto-fix). Warnings are the tracked gradual-migration backlog; do not add new errors.
  • npm run typecheck is clean (tsc --noEmit).
  • npm test -- --run passes — this runs both Vitest projects (jsdom unit + real-browser browser) and the story HTML snapshots. If you changed rendered markup on purpose, update snapshots with npm test -- --run -u and review the diff.
  • npm run build succeeds (type-check + Vite library build + .d.ts + entry shims).

Behavior & docs:

  • First-time contributors: sign the CLA (a bot will comment with instructions).
  • Added/updated Vitest + React Testing Library tests for the change (template: components/button/__tests__). New props and bug fixes should get a regression test. Tests that genuinely need a real browser (focus traversal, layout/measurement, canvas) go in a *.browser.test.jsx file (the browser project); everything else is jsdom. N/A: docs only.
  • Reviewed the component's Storybook stories: npm run storybook → http://localhost:6007/. Confirm the story actually renders and behaves (deprecated Storyshots/Karma are gone; there is no separate browser-tests server). N/A: docs only.
  • Story/example code uses the component's real prop API (e.g. render-props like onRenderContent / onRenderVisualPicker, icon props, events), not assumptions — a common source of "component looks broken" reports has been stories authored against the wrong API. Cross-check against components/<name>/__examples__. N/A: docs only.
  • Storybook Accessibility (a11y) panel shows no new violations for the affected stories. N/A: docs only.
  • If you added or changed a type, remember runtime PropTypes were removed in the TS migration — types are the contract. Ship accurate .d.ts (they're generated by the build); JS consumers rely on them via editor type-acquisition. N/A: docs only.

REVIEWER checklist (do not remove)

  • CI is green (GitHub Actions CI workflow: lint, typecheck, test, build). Chromatic visual review, if configured, has been checked.
  • Change has adequate test coverage; bug fixes include a regression test.
  • Reviewed the affected Storybook stories and their a11y panels.
  • Rendered markup still conforms to SLDS — check the story HTML snapshot diff (components/__tests__/__snapshots__/story-snapshots.test.jsx.snap).
  • Public API / package surface: if exports changed, the deep-import contract still holds (@salesforce/design-system-react/components/<name> and nested paths resolve for ESM, CJS, and types). Removing/renaming a public component or export is a breaking change → feat!:/BREAKING CHANGE: and a major version bump.

Installing @salesforce-ux/design-system-2 doesn't serve node_modules over
HTTP, so the old <link> example left components unstyled. Lead with a
bundler import, keep a serve-and-link alternative, make Cosmos the
default theme, and note the ../../public/ images the 2.264.x package
doesn't ship.

Follow-up to #3236.
- alpha.2 already published with --tag alpha; alpha.3 made it the
  default via publishConfig.tag
- Not every component became a function component
- List the SLDSMenuPicklist alias and the SLDS-prefixed aliases removed
  alongside each component

Follow-up to #3236.
- forms has a nested story and test; drop it from the gap lists
- List nested stories the snapshot glob skips
- The popper-shape adapter is computeFloatingData in utilities/dialog
- VisualPicker already supports non-coverable content
- Chromatic builds Storybook; what's missing is a public docs site

Follow-up to #3236.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant