fix(SDK-818): normalize className prop support across all SDK components - #2738
Open
dmortal wants to merge 8 commits into
Open
fix(SDK-818): normalize className prop support across all SDK components#2738dmortal wants to merge 8 commits into
dmortal wants to merge 8 commits into
Conversation
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>
…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
approved these changes
Sep 11, 2026
serikjensen
left a comment
Member
There was a problem hiding this comment.
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 viaclassNames) - Thread down to an inner
*View/*Presentationcomponent - 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 siblingDialog/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):
PayrollOverviewPropsandPayrollReceiptsPresentationPropsaddclassName?: stringwithout 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
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
CommonComponentInterfacedeclaresclassName?: stringon 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 extendingCommonComponentInterface/BaseComponentInterfacefound 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):
Each fix follows one of two established patterns already used elsewhere in the codebase:
classNameand apply it viaclassNames(styles.x, className)on the component's own wrapper, or forward it to an already-compliant child/primitive.classNamesupport to an inner*View/*Presentationcomponent that previously had none, then thread it down from the outer component.A regression test was added per fixed component, asserting the custom
classNamereaches the rendered root (following the existing test convention used inBox.test.tsx/PayrollBlockerList.test.tsx).Test plan
npm run test -- --run→ 369 files / 4032 tests passing (1 pre-existing, unrelated expected-fail)npx tsc --noEmitcleaneslintclean on all touched files (3 pre-existing, unrelatedreact-hooks/exhaustive-depswarnings confirmed present onmainbefore this change)npm run buildsucceeds🤖 Generated with Claude Code