Skip to content

fix(accordion): only use a string summary as the panel title - #3253

Open
zigzagdev wants to merge 2 commits into
salesforce:masterfrom
zigzagdev:fix/accordion-panel-title
Open

zigzagdev wants to merge 2 commits into
salesforce:masterfrom
zigzagdev:fix/accordion-panel-title

Conversation

@zigzagdev

@zigzagdev zigzagdev commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #

Additional description

Symptom

When AccordionPanel gets a JSX summary and no title, the truncated summary span renders title="[object Object]".
Hovering the panel heading shows "[object Object]" as the native tooltip.

Root cause

The span used title={title || (summary as string)}. summary accepts any ReactNode, and the cast only changes the type, not the value.
React then stringifies the element object into the attribute.
The cast also hid the type error, and every story passes a string summary, so neither typecheck nor the story snapshots caught it.
This is not a regression from #3195. The class component before the rewrite had the same fallback.

Change

summary is used as the title only when it is a string.
A JSX summary now renders no title unless the title prop is given. String summaries behave exactly as before, so there is no snapshot change and no breaking change.

  • before
スクリーンショット 2026-10-24 18 25 43
  • after
スクリーンショット 2026-10-24 18 27 13

Tests

Added three cases to accordion.test.jsx.

  • A string summary becomes the title.
  • A JSX summary renders no title.
  • The title prop wins over a JSX summary.

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.
  • 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).
  • 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__.
  • Storybook Accessibility (a11y) panel shows no new violations for the affected stories.
  • 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.

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.

AccordionPanel set `title={title || (summary as string)}` on the
truncated summary span. `summary` accepts any ReactNode, and the cast
only changes the type, not the value. When a JSX summary is passed
without `title`, React stringifies the element and renders
`title="[object Object]"`.

Fall back to `summary` only when it is a string. A JSX summary now
renders no title unless the `title` prop is given.
Checks that a string summary is used as the title, a JSX summary
renders no title, and the `title` prop wins over a JSX summary.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant