fix: refactor Button and IconButton to children-based API#1281
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the library’s custom Button and IconButton components to use a children-based content API (instead of label / leadingIcon / trailingIcon) and updates downstream usages (TopBar, Toast, FileUploadArea, stories) accordingly. It also changes TanStack Router link usage to be passed via linkOptions and bumps the package version.
Changes:
- Refactor
ButtonandIconButtonto render content viachildren, and introducelinkOptionsfor router-link rendering. - Update component call sites and Storybook stories to the new API (including icon rendering via
@equinor/eds-core-reactIcon). - Add new content wrappers / adjust button styles to support the new children-based layout and loading state.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/organisms/TopBar/Resources/ReleaseNotesDialog/ReleaseNotesDialog.tsx | Updates link button usage to linkOptions + children text. |
| src/organisms/TopBar/Account/ImpersonateMenu/DeleteUser/DeleteUser.tsx | Updates action buttons to children-based content and inlines EDS Icon. |
| src/organisms/TopBar/Account/Account.tsx | Updates logout button to children-based content and inlines EDS Icon. |
| src/organisms/SideSheet/SideSheet.stories.tsx | Migrates Storybook usage from label to children. |
| src/organisms/ReleaseNote/ToggleExpanded.tsx | Migrates toggle button to children-based label + icon. |
| src/organisms/Card/Card.stories.tsx | Migrates card action buttons from label to children. |
| src/molecules/Toast/Toast.tsx | Migrates toast action button from label to children. |
| src/molecules/RichTextEditor/custom-extensions/Counter.tsx | Migrates example button content from label to children with interpolation. |
| src/molecules/FileUploadArea/FileUploadArea.tsx | Migrates “Browse files” button to children-based icon + text. |
| src/molecules/Divider/Divider.stories.tsx | Migrates story usage from label to children. |
| src/molecules/Dialog/DialogAction.tsx | Migrates dialog action to children-based icon + text rendering. |
| src/molecules/Confetti/Confetti.stories.tsx | Migrates story usage from label to children. |
| src/molecules/Button/types.ts | Adjusts button common types (ref typing change). |
| src/molecules/Button/IconButton/IconButton.tsx | Refactors link handling to linkOptions and updates typing strategy. |
| src/molecules/Button/IconButton/IconButton.stories.tsx | Updates icon button link stories to use linkOptions. |
| src/molecules/Button/ContentWrapper.tsx | Adds new wrappers to layout children-based button content. |
| src/molecules/Button/Button.tsx | Refactors button API to children and linkOptions; removes label/icon props. |
| src/molecules/Button/Button.styles.ts | Updates styles to align with children-based rendering and loading overlay. |
| src/molecules/Button/Button.stories.tsx | Updates stories to use children-based API and linkOptions. |
| src/molecules/Banner/Banner.stories.tsx | Updates banner story to use children-based button. |
| package.json | Bumps package version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arkadiy93
commented
May 7, 2026
arkadiy93
commented
May 7, 2026
yunarch
reviewed
May 7, 2026
mariush2
previously approved these changes
May 7, 2026
Contributor
mariush2
left a comment
mariush2
left a comment
There was a problem hiding this comment.
Saw Adrian's comment about the Children usage and it sort of makes senes to me. Might be worth looking at changing it to something that isn't considered legacy by react
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/molecules/Button/Button.styles.ts:93
CenteredContentisposition: absolutewithwidth/height: 100%but no inset (top/left/right/bottom). This can leave the loading indicator positioned at its static position rather than overlaying the full button.
Suggested fix: set inset: 0 (or at least top: 0; left: 0) so the loader reliably centers over the button content.
export const CenteredContent = styled.span`
position: absolute;
width: 100%;
height: 100%;
display: flex;
yunarch
approved these changes
May 11, 2026
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.
Azure DevOps links
Task