Bugfix/CopyButton avoid layout shift for multi lined texts #3788
Conversation
…ed and a shorter feedback text is shown Also left-align and vertically center the text to look better for multi-line texts
…ginal text breaks over two lines but the confirmation does not
🦋 Changeset detectedLatest commit: f885732 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Storybook demo / Chromatic📝 Changes to review: 18c32b33b7 | 91 components | 135 stories |
| useEffect(() => { | ||
| if (buttonRef?.current) { | ||
| setFixedSize({ | ||
| height: buttonRef?.current.offsetHeight, | ||
| width: buttonRef?.current.offsetWidth, | ||
| }); | ||
| } | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
This code only runs on mount now, so screens that resizes etc will have a "wrong" sized button. Should this sizing be updated onClick/when button is active and be reset when not active?
There was a problem hiding this comment.
I can try that. 🤔
@navikt/core/css/copybutton.css
Outdated
There was a problem hiding this comment.
This change is only for "legacy" system, and not new darkside since we just use Button in "new" CopyButton
There was a problem hiding this comment.
This change only handles when the active text is shorter/fewer lines than the default text. In some cases one can have a longer active text and with this update that will break
// works
<CopyButton activeText="short text" text="long text" />
// Does not work
<CopyButton activeText="long text" text="short text" />
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a layout shift issue in the CopyButton component when dealing with multi-lined texts by preserving the button's dimensions after its first render.
- Introduces a fixed size measurement of the button using refs and state.
- Applies new inline styles to both the Button and button elements to prevent UI jump.
- Updates storybook and CSS to support the new layout behavior.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| @navikt/core/react/src/copybutton/CopyButton.tsx | Implements fixed size handling with mergeRefs and inline styles; an extraneous class "tull" is included. |
| @navikt/core/react/src/copybutton/CopyButton.stories.tsx | Adds a story to showcase the layout fix for multi-lined texts. |
| @navikt/core/css/copybutton.css | Adjusts layout properties to align content correctly. |
| .changeset/ready-points-say.md | Updates changeset documentation for the bug fix. |
Comments suppressed due to low confidence (1)
@navikt/core/react/src/copybutton/CopyButton.tsx:193
- [nitpick] The literal 'tull' appears to be an unintended class name. Remove it or replace it with the appropriate class to avoid potential confusion.
"tull",
Co-authored-by: Ken <[email protected]>
Description
On the new token page, we have a CopyButton for copying the tokens. In some cases, the text wraps over two lines.
However, when copying, the active text is single-line, causing the UI to "jump". This PR fixes that by adding a minHeight and minWidth after first render.
Component Checklist 📝
@navikt/core/css/config/_mappings.js)@navikt/core/css/tokens.json)@navikt/aksel-stylelint/src/deprecations.ts)@navikt/core/react/src/index.tsand@navikt/core/react/package.json)@navikt/core/css/index.css)<Component>: <gitmoji?> <Text>.E.g. "Button: ✨ Add feature xyz.")