feat(popover-container): align component with fusion designs - #8100
feat(popover-container): align component with fusion designs#8100edleeks87 wants to merge 1 commit into
Conversation
c3a53ca to
7788a03
Compare
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Aligns PopoverContainer with Fusion designs.
Changes:
- Migrates controls to the next-generation
Button. - Updates Fusion styling and deprecates legacy alignment/radius options.
- Adds localized default trigger text and updates tests/stories.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
skills/carbon-react/components/popover-container.md |
Excluded from review by policy. |
src/locales/locale.ts |
Defines the new translation key. |
src/locales/en-gb.ts |
Adds the English trigger label. |
src/components/popover-container/popover-container.test.tsx |
Updates and extends tests. |
src/components/popover-container/popover-container.style.ts |
Applies Fusion tokens and layout. |
src/components/popover-container/popover-container.stories.tsx |
Migrates examples to next Button. |
src/components/popover-container/popover-container.mdx |
Documents the translation key. |
src/components/popover-container/popover-container.component.tsx |
Updates controls, types, and defaults. |
src/components/popover-container/popover-container-test.stories.tsx |
Updates test stories. |
src/components/popover-container/popover-container-interaction.stories.tsx |
Migrates interaction stories. |
src/components/button/__next__/button.style.ts |
Adds legacy-menu styling overrides. |
Files excluded by content exclusion policy (1)
- skills/carbon-react/components/popover-container.md
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export interface RenderOpenProps { | ||
| tabIndex: number; | ||
| isOpen?: boolean; |
| children: ( | ||
| <> | ||
| {locale.popoverContainer.openButton.text()} | ||
| <Icon type="ellipsis_vertical" /> | ||
| </> |
| popoverContainer: { | ||
| openButton: { | ||
| text: () => "Actions", | ||
| }, | ||
| }, |
7788a03 to
19c4d1d
Compare
| "data-popover-container-button": "true", | ||
| children: ( | ||
| <> | ||
| {locale.popoverContainer.openButton.text()} |
There was a problem hiding this comment.
I'm on the fence a bit with whether context is the right approach here. In an ideal world I'd tear all of this out and just let them pass whatever button they need without the render prop but we need to avoid breaking changes right now. The alternative is to have Actions hardcoded and then consumers use the render prop to override the text etc
19c4d1d to
9ee56d4
Compare
| /** | ||
| * @deprecated | ||
| */ | ||
| tabIndex: number; |
There was a problem hiding this comment.
suggestion: currently we place the props from RenderOpenProps in an args table in the corresponding mdx file. However, this deprecation annotation doesn't show up as I believe there needs to be some sort of prop description to ensure the annotation shows up in storybook.
I think it would be worth adding a short prop description here to ensure the annotation is visible.
There was a problem hiding this comment.
Also, a bit of a related follow-up question here, I see tabIndex is not marked as deprecated for renderClose, is this intentional?
| aria-haspopup={ariaHasPopup} | ||
| aria-expanded={ariaExpanded} | ||
| id={id} | ||
| variant="default" |
There was a problem hiding this comment.
suggestion: I don't think we need to set a variant of "default" as the component has an implicit default variant
| type Position = | ||
| | "left" | ||
| | "right" | ||
| /** @deprecated "center" is deprecated and will be removed in a future release */ |
There was a problem hiding this comment.
suggestion (non-blocking): I don't believe adding a deprecated annotation like this results in any noticeable difference in an IDE and in storybook.
However, it does create some distinction from a code documentation standpoint if you wanted to leave it in.
64b34f4 to
22ebec7
Compare
565a825
22ebec7 to
565a825
Compare
a0cbb8e
565a825 to
a0cbb8e
Compare
6a1e2b0
a0cbb8e to
6a1e2b0
Compare
6a1e2b0 to
e61eafb
Compare
Proposed behaviour
Aligns with new PopoverContainer fusion designs
Uses
nextButtonDeprecates
centeralignmentDeprecates border radius prop
Adds translation key for default
openButton.text()Can be overridden by passing children directly as and when needed to open button render prop
Current behaviour
Not aligned with fusion
default open button is icon only and uses deprecated Button and IconButton
Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions