Skip to content

fix(SDK-818): normalize className prop support across all SDK components - #2738

Open
dmortal wants to merge 8 commits into
mainfrom
fix/SDK-818-normalize-classname-support
Open

fix(SDK-818): normalize className prop support across all SDK components#2738
dmortal wants to merge 8 commits into
mainfrom
fix/SDK-818-normalize-classname-support

Conversation

@dmortal

@dmortal dmortal commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

CommonComponentInterface declares className?: string on every SDK component's props so partners can override the wrapper element's CSS class, but support was inconsistent — many components accepted the prop in their type while silently dropping it. This normalizes the contract across the SDK: a fresh, independently-verified audit of every component extending CommonComponentInterface/BaseComponentInterface found 54 non-compliant components; this PR fixes all of them (146 files touched total, including inner view/presentation layers and tests) and leaves the ~40 legitimate Flow-orchestrator exemptions (no single wrapper to attach a class to) untouched.

Landed as 6 commits grouped by domain (each independently reviewable):

  • Company/Contractor
  • Contractor/Employee (Compensation, Dashboard, Documents)
  • Employee/InformationRequests/Payroll (Lists, Addresses, Terminations)
  • Payroll off-cycle/transition creation
  • Payroll PrintChecks/Overview/Receipts
  • TimeOff

Each fix follows one of two established patterns already used elsewhere in the codebase:

  • Same-file / forward: destructure className and apply it via classNames(styles.x, className) on the component's own wrapper, or forward it to an already-compliant child/primitive.
  • Threaded: add className support to an inner *View/*Presentation component that previously had none, then thread it down from the outer component.

A regression test was added per fixed component, asserting the custom className reaches the rendered root (following the existing test convention used in Box.test.tsx / PayrollBlockerList.test.tsx).

Test plan

  • Full suite: npm run test -- --run → 369 files / 4032 tests passing (1 pre-existing, unrelated expected-fail)
  • npx tsc --noEmit clean
  • eslint clean on all touched files (3 pre-existing, unrelated react-hooks/exhaustive-deps warnings confirmed present on main before this change)
  • npm run build succeeds

🤖 Generated with Claude Code

dmortal and others added 6 commits September 9, 2026 13:43
Several components extending CommonComponentInterface/BaseComponentInterface
accepted className in their props type but never applied it to a wrapper
element, so partner overrides were silently dropped.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Continues the className normalization pass across components that
accepted the prop but never applied or forwarded it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… components

Continues the className normalization pass across components that
accepted the prop but never applied or forwarded it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Continues the className normalization pass across components that
accepted the prop but never applied or forwarded it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s components

Continues the className normalization pass across components that
accepted the prop but never applied or forwarded it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completes the className normalization pass (SDK-818) across all SDK
components extending CommonComponentInterface/BaseComponentInterface.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dmortal
dmortal requested a review from a team as a code owner September 9, 2026 20:06
github-actions Bot and others added 2 commits September 9, 2026 20:10
…ot instead

The earlier commits in this branch normalized className support by wrapping
many components' Flex-rooted content in a brand-new <div className={className}>.
That extra div is an unstyled flex item inside components like <Form>
(display:flex; align-items:start), and since every descendant below it sizes
itself via width:100%, the div's own intrinsic width collapses to 0 — hiding
the entire subtree (caught by a real CI e2e failure on the Transition flow's
Check date field, reproduced and confirmed via Storybook/local dev server).

Fixes it at the source: Flex now accepts and forwards className to its own
root element, so every affected component applies className directly to its
existing <Flex> instead of introducing a new wrapping element. A few callers
that wrapped a different internal *Presentation component instead of Flex
directly are fixed the same way — className threads onto that component's
own existing styled root.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@serikjensen serikjensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all 153 files with a focus on className application. The change is consistent and mechanical throughout. Each fix uses one of the established patterns:

  • Forward to Flex (<Flex className={className} ...>, merged via classNames)
  • Thread down to an inner *View/*Presentation component
  • Merge onto an existing styled root (classNames(styles.container, className))
  • Forward to a primitive that already accepts it (Form, HtmlForm, Alert, Select)
  • New <section className={className}> wrapper where the component returns a fragment with a sibling Dialog/Modal

Every fixed component has a matching regression test asserting the class reaches the rendered root. No exceptions to the contract found.

Two trivial, non-blocking nits (not required to address):

  • PayrollOverviewProps and PayrollReceiptsPresentationProps add className?: string without the /** CSS class name applied to the root element. */ doc comment used elsewhere. Both are @internal, so it's cosmetic.
  • The <section> boundary differs slightly between the address views (modals inside) and the employee-list views (dialog outside), but this is irrelevant since dialogs render in portals.

LGTM.


Generated by Claude Code

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.

2 participants