Bk/bug/5511 fix copy icon btn#5552
Conversation
Signed-off-by: Bart Kuijper <bart.kuijper89@gmail.com>
Signed-off-by: Bart Kuijper <bart.kuijper89@gmail.com>
|
Thanks for this change. It is the difference between getting something in place and making it look good. There are three behavior regressions I'd like fixed before merge, plus two smaller ones. All are small changes. 1. Click propagation needs to be stopped again. 2. The failure state is terminal — give it a way back. 3. Guard against 4. Space key: keep the 5. Restore the There's a second, project-specific reason to keep ARIA attributes healthy here. The theme-builder tool under Smaller things, take or leave:
None of this is big — the direction is good and I'd like to merge it once the propagation, error-reset, and clipboard-guard points are addressed. |
Description
Fix copy-to-clipboard icon behavior and state transitions
Motivation and Context
This PR refactors the copy-to-clipboard component to use a clear state-driven UI flow.
How Has This Been Tested?
Test the temporal nature of the component as well as the error state imagined for the off change that corporate's (clients) that somehow block copying ability. Followed by the fact that the
(method) Clipboard.writeText(data: string): Promise<void>is a promise hence has a catch implied by its implementation by the browser(s)
besides automated unit test (with integration towards virtual dom nodes)
There is a manual test instruction in the form of addinga runtime reassignment of a method to mock an error state of the clipboard. Beyond that it has been tested on all modern browsers.
Types of changes
It uses modern angular approach instead of conditionally hiding elements based on opacity. This is proved fragile with bubbling events. This clean approach can also be extended into the realm of lazy loading so that chunks are reeled in async however (defer syntax) for now that is kept out of scope due to the limited code.
Also the green iconography has been added with animation. New animation was created where a key approach was to keep the scale equal or under 1 to assure it never clips into its parent div.
Also a conditional animation was added to the clipboard icon to satisfyingly come back from it all.
also the @input decorator was used with required: true to give compile time type safety in the components. However due to the strictness level actual the compilation is waived for now. There is a ticket with more info and some scripts: #5505
Lastly the parent element has been shifted to more natural fit the baseline of the text. Also it has been moved ever so slightly and the width is now determined by its content.
There are a couple of side effects in the code which can be avoided if rewritten entirely with RxJs however it was deemed more readable to rely on promises and signal state.
Since zone.js was removed no more need for on push setting
Checklist: