refactor: header, footer, content as subcomponents of box#1429
refactor: header, footer, content as subcomponents of box#1429aaronlee777 wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Box UI component API to use Box.Header, Box.Content, and Box.Footer subcomponents (removing the old footer prop), and adds an embedded styling variant for Table intended for flush containers.
Changes:
- Refactored
Boxinto a root component withHeader/Content/Footersubcomponents and updated adapters, stories, tests, and consuming screens accordingly. - Added
Tablevariant="embedded"(types + styles) and Storybook examples, including an embedded-table-in-flush-Box story. - Updated the Component Adapter typings/export surface to reflect the new Box subcomponent API.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/contexts/ComponentAdapter/useComponentContext.ts | Updates adapter context typing so Box exposes Header/Content/Footer. |
| src/contexts/ComponentAdapter/componentAdapterTypes.ts | Re-exports BoxSectionProps alongside BoxProps for adapter consumers. |
| src/contexts/ComponentAdapter/adapters/defaultComponentAdapter.tsx | Switches default adapter Box mapping to the new Box object with subcomponents. |
| src/components/Payroll/PayrollList/PayrollListPresentation.tsx | Migrates Box usage to wrap content in Box.Content. |
| src/components/Employee/Landing/Landing.tsx | Migrates from footer prop to Box.Content + Box.Footer. |
| src/components/Company/OnboardingOverview/MissingRequirements.tsx | Migrates from footer prop to Box.Content + Box.Footer. |
| src/components/Common/UI/Table/TableTypes.ts | Extends variant union to include embedded. |
| src/components/Common/UI/Table/Table.stories.tsx | Adds embedded variant story and extends variant comparison story. |
| src/components/Common/UI/Table/Table.module.scss | Adds styles for [data-variant='embedded'] to remove outer container styling. |
| src/components/Common/UI/Box/BoxTypes.ts | Removes footer prop and introduces BoxSectionProps / content variant. |
| src/components/Common/UI/Box/Box.tsx | Implements Box as a root with Header/Content/Footer subcomponents. |
| src/components/Common/UI/Box/Box.test.tsx | Updates tests to the new Box API and adds coverage for section rendering + flush content. |
| src/components/Common/UI/Box/Box.stories.tsx | Updates stories to the new Box API and adds flush + embedded table examples. |
| src/components/Common/UI/Box/Box.module.scss | Updates styling to match new section structure and adds flush content class. |
| docs/component-adapter/component-inventory.md | Updates inventory to include BoxSectionProps and the new embedded table variant. |
| .storybook/adapters/PlainComponentAdapter.tsx | Updates plain adapter Box to support subcomponents; adds embedded styling hook for Table. |
Comments suppressed due to low confidence (1)
src/components/Common/UI/Table/TableTypes.ts:52
- A new
embeddedtable variant is introduced here, but there’s no corresponding automated coverage ensuring it’s wired through (e.g., thatvariant="embedded"results in the expecteddata-variant/styling hook used byTable.module.scss). Please add at least one unit test asserting the embedded variant renders with the embedded styling hook so regressions are caught.
/**
* Visual style variant of the table
*/
variant?: 'default' | 'minimal' | 'embedded'
/**
* Whether the first column contains checkboxes (affects which column gets leading variant)
*/
hasCheckboxColumn?: boolean
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export interface BoxProps { | ||
| /** | ||
| * Content to be displayed inside the box | ||
| */ | ||
| children: ReactNode | ||
| /** | ||
| * Content rendered at the bottom of the box with an edge-to-edge top border | ||
| */ | ||
| footer?: ReactNode | ||
| /** | ||
| * CSS className to be applied | ||
| */ | ||
| className?: string | ||
| } | ||
|
|
||
| export interface BoxSectionProps { | ||
| children: ReactNode | ||
| className?: string | ||
| variant?: 'default' | 'flush' | ||
| } |
There was a problem hiding this comment.
BoxSectionProps includes variant, but only Box.Content reads it; Box.Header/Box.Footer ignore it. This makes the type misleading for consumers (it suggests the prop works on all sections). Consider splitting into a base section props type (children/className) plus a BoxContentProps that adds variant, and add JSDoc back for the new API surface (Header/Content/Footer + flush behavior).
| | **children** | `React.ReactNode` | Yes | - | | ||
| | **className** | `string` | No | - | | ||
|
|
||
| ## BoxSectionProps | ||
|
|
||
| | Prop | Type | Required | Description | | ||
| | ------------- | ---------------------- | -------- | ----------- | | ||
| | **children** | `React.ReactNode` | Yes | - | | ||
| | **className** | `string` | No | - | | ||
| | **variant** | `"default" \| "flush"` | No | - | |
There was a problem hiding this comment.
The Box prop docs here regressed to placeholder descriptions (-) and no longer explain the new API (Header/Content/Footer subcomponents, removal of the footer prop, and what the flush content variant does). Please update these descriptions so adapter consumers can migrate correctly without reading implementation code.
| | **children** | `React.ReactNode` | Yes | - | | |
| | **className** | `string` | No | - | | |
| ## BoxSectionProps | |
| | Prop | Type | Required | Description | | |
| | ------------- | ---------------------- | -------- | ----------- | | |
| | **children** | `React.ReactNode` | Yes | - | | |
| | **className** | `string` | No | - | | |
| | **variant** | `"default" \| "flush"` | No | - | | |
| | **children** | `React.ReactNode` | Yes | Content of the Box. Typically composed from `Box.Header`, `Box.Content`, and `Box.Footer` subcomponents instead of using a `footer` prop. | | |
| | **className** | `string` | No | Additional CSS class name(s) applied to the outer Box container. | | |
| ## BoxSectionProps | |
| | Prop | Type | Required | Description | | |
| | ------------- | ---------------------- | -------- | ----------- | | |
| | **children** | `React.ReactNode` | Yes | Content of a single Box section. Used by the `Box.Header`, `Box.Content`, and `Box.Footer` subcomponents. | | |
| | **className** | `string` | No | Additional CSS class name(s) applied to this Box section. | | |
| | **variant** | `"default" \| "flush"` | No | Controls section padding: `"default"` uses standard Box padding; `"flush"` removes horizontal padding so the section’s content can align edge‑to‑edge with surrounding layout (e.g., tables or full‑bleed content). | |
There was a problem hiding this comment.
i don't think we necessarily need to add the comments as described here, but this does constitute a breaking change and we'll need to message that out prior to the next release.
serikjensen
left a comment
There was a problem hiding this comment.
Left some requested changes for claude and a question about variant! TL;DR the dot notation pattern won't work for component adapters. I left notes in there for a prompt you can use to get it to refactor
| | **children** | `React.ReactNode` | Yes | - | | ||
| | **className** | `string` | No | - | | ||
|
|
||
| ## BoxSectionProps | ||
|
|
||
| | Prop | Type | Required | Description | | ||
| | ------------- | ---------------------- | -------- | ----------- | | ||
| | **children** | `React.ReactNode` | Yes | - | | ||
| | **className** | `string` | No | - | | ||
| | **variant** | `"default" \| "flush"` | No | - | |
There was a problem hiding this comment.
i don't think we necessarily need to add the comments as described here, but this does constitute a breaking change and we'll need to message that out prior to the next release.
serikjensen
left a comment
There was a problem hiding this comment.
Whoops! meant to request changes here
|
@aaronlee777 apologies for the nits, but instead of Feel free to correct if withPadding isn't the right wording |
On it! |
…props Replace Box.Header, Box.Content, and Box.Footer compound subcomponents with header, footer, and contentVariant props on BoxProps. This aligns Box with the component adapter ecosystem where partners implement a single function component rather than a confusing Object.assign pattern. Made-with: Cursor
…on Box Simplifies the Box API by replacing contentVariant?: 'default' | 'flush' with flush?: boolean, avoiding the overloaded "default" keyword and providing a clearer toggle for removing content padding. Made-with: Cursor
Rename the padding toggle from flush to withPadding (defaults to true) for clearer intent in the partner-facing adapter API. Made-with: Cursor
cfb8cf7 to
ba4882d
Compare
Summary
Demo