UI: Add copy button for client/allocation UUIDs #5926
Conversation
I found while working on #5926 that x-icon already adds assertion-compatible selectors, so these wrappers are unnecessary.
Thanks to @eshtadc for this article: https://dockyard.com/blog/2018/04/18/bending-time-in-ember-tests
This shouldn’t really be a button, I just wanted it to have the same spacing and size as the button.
I’m trying out this UX as seen on GitHub PR pages. Easily removed if undesirable!
If there are no uses where it should be hidden until hover, I’ll just entirely remove that.
I found while working on #5926 that x-icon already adds assertion-compatible selectors, so these wrappers are unnecessary.
If this is ever needed somewhere, it’s easily brought back.
Thanks to @lukemelia for this suggestion on handling component name overlaps: emberjs/ember.js#17997 (comment)
backspace
changed the title
ui/app/components/copy-button.js
Outdated
| this.set('state', 'success'); | ||
|
|
||
| run.later(() => { | ||
| this.set('state', null); |
There was a problem hiding this comment.
I think we need a this.isDestroyed check here. Otherwise, if the user navigates away during this 2 second timeout, we'll try to set a value on a destroyed object.
(This is one reason I really like using ember-concurrency for things like this: free task cancelation on unrender.)
There was a problem hiding this comment.
Thanks for the pointer, I converted it to a task, and gave it a more descriptive name while I was at it.
ui/app/components/copy-button.js
Outdated
|
|
||
| yield run.later(() => { | ||
| this.set('state', null); | ||
| }, 2000); |
There was a problem hiding this comment.
Unfortunately, I think this implementation is still subject to the same bug. I just reproduced it on the Netlify preview, at least:
Here's one way to avoid the issue:
import { task, timeout} from 'ember-concurrency';
export default Component.extend({
// …
indicateSuccess: task(function*() {
this.set('state', 'success');
yield timeout(2000);
this.set('state', null);
}).restartable(),
});This works because on cancelation, the timeout is aborted, and the rest of the task is not executed.
Edit: the run.later approach doesn't work because it's not cancellable: once the work is scheduled on the queue, you can't remove it.
(I also added restartable to make sure we don't have multiple concurrent version of this task running if the user clicks a few times.)
There was a problem hiding this comment.
goodness, yes, much better, thanks!
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
You can try this by visiting the preview deployment client list and choosing a client (direct linking isn’t possible because of randomised Mirage data).
Here’s a GIF of it for an allocation UUID: