Successfully copying changes the icon to a tick #1404
Conversation
JamesNK
requested review from
drewnoakes,
smitpatel,
tlmii,
adamint and
kvenkatrajan
JamesNK
changed the title
JamesNK
force-pushed
the
jamesnk/copy-tick
branch
from
188b075
to
0074b70
Compare
| // If there is a pending timeout then clear it. Otherwise the pending timeout will prematurely reset values. | ||
| if (button.dataset.copyTimeout) { | ||
| clearTimeout(button.dataset.copyTimeout); | ||
| delete button.dataset.copyTimeout; |
There was a problem hiding this comment.
copyTimeout is just a number (specifically an integer), do we need to delete it? setting it to null, undefined or even 0 would probably still work. I'm curious if this is something specific for the result of setTimeout or not.
There was a problem hiding this comment.
Don't need to use delete. But I did because button.dataset is basically a dictionary, and delete is the closest thing to .NET's dictionary.Remove(key).
| }) | ||
| .catch(() => { | ||
| tooltipDiv.innerText = 'Could not access clipboard'; | ||
| }); | ||
| setTimeout(function () { tooltipDiv.innerText = precopy }, 1500); | ||
|
|
||
| button.dataset.copyTimeout = setTimeout(function () { |
There was a problem hiding this comment.
Could you explain why all this code is JS and not blazor?
There was a problem hiding this comment.
I went with the existing code (changing the tooltip title) that already existed in JS.
I thought about doing everything in Blazor but there is UI lag to consider. If the dashboard is in ACA and you have a slow connection, then there could be a 1s+ delay between clicking copy and the confirmation that content was copied. JS will always be fast.
JamesNK
force-pushed
the
jamesnk/copy-tick
branch
from
0074b70
to
b1e0886
Compare
Co-authored-by: Drew Noakes <[email protected]>
Fixes #1386
Microsoft Reviewers: Open in CodeFlow