feat: add copy to clipboard as valid item type for button group#4736
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4736 +/- ##
==========================================
+ Coverage 97.60% 97.61% +0.01%
==========================================
Files 951 952 +1
Lines 30744 30816 +72
Branches 11281 11317 +36
==========================================
+ Hits 30007 30081 +74
- Misses 690 728 +38
+ Partials 47 7 -40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| const onCopyFailureHandler: NonCancelableEventHandler<CopyToClipboardProps.CopyFailureDetail> = event => { | ||
| fireCancelableEvent(onCopyFailure, event.detail); | ||
| setTooltip(null); |
There was a problem hiding this comment.
This does not really work. The tooltip is hidden initially, but then it re-appears if the item is hovered.
In button group we use a customised tooltip implementation that transitions from tooltip to popover state if needed - that was used precisely to implement copy-to-clipboard items (which required a bit of extra code to actually do copy to clipboard).
I think reusing the copy to clipboard component here does not really work - it is best to use a custom implementation but reuse relevant utilities from the copy to clipboard component instead.
There was a problem hiding this comment.
I see. Just want to clarify that the expected behavior is that the success/failure popover and the tooltip should render as one component as opposed to being rendered on top of the other here, which can't be accomplished with simply reusing the copy to clipboard component?
There was a problem hiding this comment.
The tooltip does not have to remain the same dom element when transitioning from tooltip to popover state, but we should not display both at the same time.
| function renderButtonGroup(props: Partial<ButtonGroupProps>) { | ||
| const { container } = render(<ButtonGroup variant="icon" ariaLabel="Chat actions" items={[copyItem]} {...props} />); | ||
| const wrapper = createWrapper(container).findButtonGroup()!; | ||
| const copyToClipboard = createWrapper(container).findCopyToClipboard()!; |
There was a problem hiding this comment.
This is wrong: the button dropdown wrapper must receive its findCopyToClipboardById() helper instead.
There was a problem hiding this comment.
is this due to the possibility that there can be multiple copy to clipboard buttons?
edit: since I'm updating this to a cusotm implementation using iconbutton, updating this to findButtonById()
There was a problem hiding this comment.
Yes, but also to ensure consistency when it comes to different item types. Even if there is only one copy button - the consumers should use the button group's utils for that - as whether we implement it by reusing the copy-to-clipboard component, or use a custom solution - is an implementation detail.
| function renderButtonGroup(props: Partial<ButtonGroupProps>) { | ||
| const { container } = render(<ButtonGroup variant="icon" ariaLabel="Chat actions" items={[copyItem]} {...props} />); | ||
| const wrapper = createWrapper(container).findButtonGroup()!; | ||
| const copyToClipboard = wrapper.findButtonById('copy')!; |
There was a problem hiding this comment.
We need to introduce findCopyToClipboardById() util. This is needed for consistency but also it gives consumers the ability to validate item's type this way.
Head branch was pushed to by a user without write access
2 participants
Description
Add
icon-copy-to-clipboardas a valid item type in the ButtonGroup component. Thisallows users to embed a copy-to-clipboard action directly within a button group
alongside existing item types (icon-button, icon-toggle-button, icon-file-input,
menu-dropdown).
Changes:
IconCopyToClipboardinterface with properties:id,text,textToCopy,copySuccessText,copyErrorText,disabled, anddisabledReasononCopySuccessandonCopyFailureevent handlers toButtonGroupPropsCopyToClipboardItemcomponent that wrapsInternalCopyToClipboardwithtooltip support
ItemRuntime,Item, andInternalItemunion types to include the newitem type
internal.tsxanditem-element.tsxRelated links, issue #, if available: n/a
How has this been tested?
icon-copy-to-clipboarditemReview checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md](https://github.com/cloudscape-design/components/blob/main/CONTRIBUTING.md#public-apis).
[
CONTRIBUTING.md](https://github.com/cloudscape-design/components/blob/main/CONTRIBUTING.md#browsers-support).
guidelines.
Security
checkSafeUrlfunction](https://github.com/cloudscape-design/components/blob/main/src/internal/util
s/check-safe-url.ts).
Testing