-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ui] Copyable server and client attribute values #16548
[ui] Copyable server and client attribute values #16548
Conversation
53aa8d2
to
9f35b7a
Compare
Ember Asset Size actionAs of d9cdb83 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ty! 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a pretty easy win to migrate CopyButton
to Glimmer while we're here.
import classic from 'ember-classic-decorator'; | ||
|
||
@classic | ||
@classNames('copy-button') | ||
@classNameBindings('inset') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
: There are only 6 invocations of the CopyButton
component. Rather than persisting ember-classic
pattern, this seems like an easy win for us to migrate to Ember Octane via a Glimmer component!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave it a shot, including upgrading ember-cli-clipboard, and ran into errors. Seems not worth holding up this feature for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it up and running pretty immediately. Which errors did you run into? Do you think we'll run into similar errors on the upgrade path to Ember 4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open a PR! All look and feel was fine but no events were firing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roger dodger!
Resolves #16539
Adds Copy buttons to server and client attribute values:
In order to get this working within the confines of a (potentially overflow:hidden) table row, added the
@inset
argument, which will modify the background to be transparent, the dom placement to be inline, and to not show a "Copied!" message and defer instead to just the checkmark icon.