<sl-copy> #1483

claviska · 2023-08-02T20:15:03Z

I jumped the gun on merging #1473. It turns out the same issues I had with my initial stab at a copy component are still concerns. I reverted the PR so we can gather more feedback. Here's what I did and what the remaining concerns are. (I'm doing this from memory so I might be forgetting some things.)

Preview

https://shoelace-git-copy-font-awesome.vercel.app/components/copy/

Changes

Renamed to <sl-copy>
Refactored some things to simplify internals and make more consistent with other components
Added feedback animation
Added localization
Added a new copy icon to the system icon library
Changed for to from
Renamed slots to success and error
Changed events a bit
Cleaned up and updated examples in the docs
Changed copy and feedback icons
Removed <sl-tooltip> as a dependency to reduce the footprint of <sl-copy>

Concerns

My biggest issue is around customization. Ideally, the component will allow users to customize the icons and/or buttons. We're accomplishing that right now using three slots.

<sl-copy value="Copy me">
  <sl-button>Copy</sl-button>
  <sl-button slot="success">Copied!</sl-button>
  <sl-button slot="error">Error</sl-button>
</sl-copy>

I know this was my suggestion, but after using it awhile, it feels like the wrong choice. The button itself shouldn't get swapped out, as it causes the trigger to lose focus and tooltips can't follow it so they disappear. It's also not intuitive to users why they need to provide three buttons when really it's the label they need to customize.

Perhaps the way forward here is to not allow arbitrary buttons, but bake the button in and allow it to be styled with CSS instead. Then we could use slots for just the icons, which feels a lot better.

<sl-copy value="Copy me">
  <sl-icon slot="copy" name="files"></sl-icon>
  <sl-icon slot="success" name="check"></sl-icon>
  <sl-icon slot="error" name="x-lg"></sl-icon>
</sl-copy>

The other thing is the animation. Cutting over without any animation felt jarring. I added a subtle one that makes it feel much better. (Excuse the choppy GIF)

But this doesn't really work for custom buttons. Well, it does, but it feels weird because it's intended for icons.

While I did make the animations customizable, I don't think they should apply to the entire slot, so baking the button in would solve this too.

TL;DR

I initially had a vision for this to be a generic utility component where you could control pretty much everything declaratively, but we might need to make it a bit more opinionated to solve these problems.

I'd also like to wait for @lindsaym-fa's feedback. Maybe having a proper design for this component will be helpful to steer it in the right direction.

I'd love to hear everyone's thoughts on this one.

/cc @daKmoR @KonnorRogers

vercel · 2023-08-02T20:15:07Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
shoelace	✅ Ready (Inspect)	Visit Preview	Aug 11, 2023 2:29pm

daKmoR · 2023-08-02T20:35:56Z

I love those improvements 👍 well done 💪

and I am fully onboard with the idea to baking the button into the copy component itself.
The possibility to replace the icon/content will still solve like 95% of the use cases.

docs/pages/components/copy.md

Co-authored-by: Thomas Allmer <[email protected]>

lindsaym-fa · 2023-08-03T19:49:29Z

Would we consider using sl-tooltip for the copied and error states, much like we do when copying icons in the icon docs?

I totally agree with not swapping out the button. In general, we probably don't want to encourage users to change the label of the button either, although they certainly still could if they wanted to. (It's one button that always performs the same action, so that context would ideally remain the same for the end user.)

claviska · 2023-08-03T20:14:12Z

From https://twitter.com/tobiasbu/status/1687187298884915200

A minor issue: 'from=id' sometimes uses innerText, value-attr, or href-attr. While the choice often fits, the inconsistency is distracting. Maybe adding 'from-attr=href' for consistency?

That's a good point. It's a bit too magical, so we can probably do this with a second attribute:

<sl-copy from="el"> (copies el.textContent by default)
<sl-copy from="el" attr="value"> (copies #el.value)
<sl-copy from="el" attr="href">  (copies #el.href)

daKmoR · 2023-08-03T21:04:30Z

githubs copy button is imho doing a good job

just as another point of reference 🤗
and I think the ui is already very similar 💪 (which is imho good)

and here is the html for it

seems another good point to go with one button...
and maybe adding a tooltip for success / error only

success-feedback (defaults to Copied!)
error-feedback (defaults to Error)

what about tooltip direction? 🤔

justinfagnani · 2023-08-04T02:27:12Z

The design problem for custom buttons seems eerily similar to that of <selectmenu> and its need to have user-provided buttons in "slots" for specific roles, but possibly even trickier because of the toggle state of the button (or maybe that's the same as the dropdown indicator button in selectmenu).

I personally think that not allowing custom buttons, but allowing custom icons, is just fine, especially how simple it is compared to the alternatives. Allowing people to easily extend sl-copy and swap out the button rendering would be an easy path to customization too.

Ultimately, customization, even for some fairly simple cases calls for a render-prop type of approach. This is because you don't know if the state of sl-copy should change attributes or children of the button or something else. Kevin on our team was working on a render-prop-like community protocol for cases like this. It would be a bit complex though.

A somewhat simpler approach might be to define a protocol and microsyntax so that sl-copy can mutate its children. You could imagine attributes on sl-copy that have a syntax for setting attribute and content of the button.

<sl-copy
    value="Copy me"
    success="icon='check'; #text='Copied!'"
    error="icon='x-lg'; #text='Error!'">
  <sl-button icon="files">Copy</sl-button>
</sl-copy>

#text is a special name to represent the text content. sl-copy should remember the initial values to reset them if needed.

Still a bit complicated though it doesn't require any JS.

Subclassing is a quite easy implement customization approach if you wanted to allow that:

import {html} from 'lit';
import {choose} from 'lit/directives/choose.js';
import {customElement} from 'lit/decorators/custom-element.js';
import {Copy} from '@shoelace-style/shoelace/dist/components/copy/copy.component.js';

@customElement('my-copy')
export class MyCopy extends Copy {
  protected renderButton() {
    return html`
      <sl-button>${choose(this.state, [
        ['ready', () => 'Copy'],
        ['copied, () => 'Copied!'],
        ['error, () => 'Error']
      ])}</sl-button>`;
  }
}

KonnorRogers · 2023-08-04T20:17:08Z

@justinfagnani For me personally, I have 0 desire to introduce a DSL for this. I think this is where slots shine and we should embrace them.

The big problem with swapping buttons is it means you lose focus state so you'd have to refocus to the new button when the new button comes in, and then once the error or success clears you need to refocus the initial button, which could be very jarring. I would say I expect buttons to be baked in. I would think the button is in an disabled state while in the success / error state anyways.

<sl-copy value="<text to be copied>">
  Copy
  
   <span slot="success">
      <sl-icon name="check"></sl-icon>
     Copied successfully
   </span>
   
   <span slot="error">
     <sl-icon name="x-lg"></sl-icon>
     Error!
   </span>
</sl-copy>

daKmoR · 2023-08-05T23:25:36Z

imho the end result should be similar to the github copy button...
therefore button and tooltip should be "built in".

users can provide content for each of those slots e.g. the maximum example would be

<sl-copy value="<text to be copied>">
  <span slot="copy-button"><sl-icon name="my-icon"></sl-icon></span>
  <span slot="copy-msg">Do it!</span>

  <span slot="success-button"><sl-icon name="my-success-icon"></sl-icon></span>
  <span slot="success-msg">You did it <confetti></span>
  
  <span slot="error-button"><sl-icon name="my-error-icon"></sl-icon></span>
  <span slot="error-msg">Nope</span>
</sl-copy>

it could also be written as attributes so the most common case will probably be to only set the success message

<sl-copy value="<text to be copied>" success-msg="You did it"></sl-copy>

there should be a default msg content for success and error and it only shows the tooltip if there is an actual message to show.

daKmoR · 2023-08-10T10:02:59Z

any ways I can help? shall I prepare a PR with my proposed changes to this PR? 🤔

I would think the button is in an disabled state while in the success / error state anyways.

just a note: disabled buttons are not focusable so they would loose the focus as well...

claviska · 2023-08-10T22:37:21Z

Thanks for the input. The latest changes seem promising. I've renamed it to <sl-copy-button> which is a bit more specific and feels better than <sl-copy>.

The labels are localized and simple by default, but you can override them using attributes to provide more context to the user (e.g. "Copy source code" instead of just "Copy").

The button is now static, and only the icons can be changed via slot. Tooltips have been added back and are used to announce the label to screen readers.

The from="el" attribute now copies el.textContent by default, but you can provide an attribute or property modifier to change that. So an attribute would be from="el[href]" and a property would be from="el.propertyName". (This is as close to a custom DSL as I'd like to get.)

<!-- Copies the span's textContent -->
<span id="my-phone">+1 (234) 456-7890</span>
<sl-copy-button from="my-phone"></sl-copy-button>


<!-- Copies the input's "value" property -->
<sl-input id="my-input" type="text" value="User input"></sl-input>
<sl-copy-button from="my-input.value"></sl-copy-button>


<!-- Copies the link's "href" attribute -->
<a id="my-link" href="https://shoelace.style/">Shoelace Website</a>
<sl-copy-button from="my-link[href]"></sl-copy-button>

Let me know what you think of the updates. If this looks good, we can probably get this into the release next week as experimental.

Latest preview: https://shoelace-ly2lsft6z-font-awesome.vercel.app/components/copy-button/

claviska · 2023-08-11T14:27:15Z

Added a couple things:

The hoist attribute passes through to the tooltip to let you prevent clipping in overflow containers
Exported the tooltip parts
Added parts for each icon container
Updated the copy icon to be more consistent

I'm going to merge this as experimental, but please keep the feedback coming!

mitchray · 2023-08-12T02:04:34Z

Just pointing out the release notes say the new component is sl-clipboard but the docs/code is sl-copy-button

Regardless I've already incorporated it into my project!

daKmoR · 2023-08-12T22:54:54Z

I really like how the current API looks 👍
nice outcome 🤗

looking forward to using it.

One minor detail - there is no simple way to disable the tooltip.
I would expect if I set it to an empty string that the tooltip would "disappear"

<sl-copy-button copy-label=""></sl-copy-button>

but this does "nothing" e.g. the default copy label is used

claviska · 2023-08-14T13:43:39Z

Just pointing out the release notes say the new component is sl-clipboard but the docs/code is sl-copy-button

I think you might be seeing an older preview. The current changelog for 2.7.0 shows the correct name, but let me know if I'm missing something.

One minor detail - there is no simple way to disable the tooltip.

The tooltip serves as the label. Without it, there would be no feedback for assistive devices. At some point, I was using labels but then it announced twice (once for the label, once for the tooltip).

If we want to support this, we'd need an attribute like no-tooltip that 1) disables the tooltip and 2) conditionally renders the labels within the button. Alas, we can't throw it on each <sl-icon> because they're fallback content that will can get replaced. It would probably need to be <button aria-label="..."> or perhaps an <sl-visually-hidden> element inside the button — and it would need to be updated dynamically with the status.

mitchray · 2023-08-15T09:02:36Z

Ah I was going by https://github.com/shoelace-style/shoelace/releases/tag/v2.7.0 but I'll stick to the main site changelog going forward

xiaohk · 2024-09-18T18:22:20Z

I really like how the current API looks 👍 nice outcome 🤗

looking forward to using it.

One minor detail - there is no simple way to disable the tooltip. I would expect if I set it to an empty string that the tooltip would "disappear"
<sl-copy-button copy-label=""></sl-copy-button>
but this does "nothing" e.g. the default copy label is used

Hi! Did you figure out a way to disable the tooltip?

copy updates

72d4e79

daKmoR reviewed Aug 2, 2023

View reviewed changes

docs/pages/components/copy.md Outdated Show resolved Hide resolved

Update docs/pages/components/copy.md

ff0183a

Co-authored-by: Thomas Allmer <[email protected]>

vercel bot deployed to Preview August 3, 2023 13:50 View deployment

claviska added 3 commits August 10, 2023 13:36

Merge branch 'next' into copy

100b639

unwrap and fix case

d6f9618

copy button updates

2e6133d

vercel bot deployed to Preview August 10, 2023 22:31 View deployment

claviska marked this pull request as ready for review August 11, 2023 14:02

claviska self-assigned this Aug 11, 2023

claviska added the feature Feature requests. label Aug 11, 2023

claviska added 4 commits August 11, 2023 10:20

use bs icon

f15d918

add parts, hoist, and improve parsing a bit

a1eb57d

update docs

6d7dcfc

remove comment

f8abd52

claviska merged commit c36df5e into next Aug 11, 2023
1 check passed

vercel bot deployed to Preview August 11, 2023 14:29 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

<sl-copy> #1483

<sl-copy> #1483

claviska commented Aug 2, 2023

vercel bot commented Aug 2, 2023 •

edited

Loading

daKmoR commented Aug 2, 2023

lindsaym-fa commented Aug 3, 2023

claviska commented Aug 3, 2023

daKmoR commented Aug 3, 2023 •

edited

Loading

justinfagnani commented Aug 4, 2023

KonnorRogers commented Aug 4, 2023

daKmoR commented Aug 5, 2023

daKmoR commented Aug 10, 2023

claviska commented Aug 10, 2023 •

edited

Loading

claviska commented Aug 11, 2023 •

edited

Loading

mitchray commented Aug 12, 2023

daKmoR commented Aug 12, 2023

claviska commented Aug 14, 2023

mitchray commented Aug 15, 2023

xiaohk commented Sep 18, 2024

<sl-copy> #1483

<sl-copy> #1483

Conversation

claviska commented Aug 2, 2023

Preview

Changes

Concerns

TL;DR

vercel bot commented Aug 2, 2023 • edited Loading

daKmoR commented Aug 2, 2023

lindsaym-fa commented Aug 3, 2023

claviska commented Aug 3, 2023

daKmoR commented Aug 3, 2023 • edited Loading

justinfagnani commented Aug 4, 2023

KonnorRogers commented Aug 4, 2023

daKmoR commented Aug 5, 2023

daKmoR commented Aug 10, 2023

claviska commented Aug 10, 2023 • edited Loading

claviska commented Aug 11, 2023 • edited Loading

mitchray commented Aug 12, 2023

daKmoR commented Aug 12, 2023

claviska commented Aug 14, 2023

mitchray commented Aug 15, 2023

xiaohk commented Sep 18, 2024

vercel bot commented Aug 2, 2023 •

edited

Loading

daKmoR commented Aug 3, 2023 •

edited

Loading

claviska commented Aug 10, 2023 •

edited

Loading

claviska commented Aug 11, 2023 •

edited

Loading