aeternity / aepp-components

deprecated: aepp-components to be used in all aepps.
ISC License
41 stars 14 forks source link

copyToClipboard: Copy actual value #255

Open davidyuk opened 5 years ago

davidyuk commented 5 years ago

closes https://github.com/aeternity/aepp-base/issues/319

davidyuk commented 5 years ago

I think tests fail because of the too old version of jest: https://github.com/jsdom/jsdom/issues/961 https://www.npmjs.com/package/jest/v/21.2.1 it can be fixed by #124

sadiqevani commented 5 years ago

So @davidyuk, I went and replicated the issue in the ticket you referenced. Now, what I see as an issue is that the data in the directive does not update upon VNode update.

Using the internal binding would be a better approach than touching the DOM, in this case now that the directive returns a function, the function will execute on both: bind, update

Wouldn't this work the same as your code?

export default (el, binding) => el.addEventListener('click', async () => {
  /**
   * In case the value is falsy, do not proceed with
   * the copyToClipboard functionality
   */
  if (!binding.value) return;

  /**
   * Await for copy to be executed and proceed
   * with normal flow
   */
  await copy(binding.value);
  el.classList.add('v-copied-to-clipboard');
  setTimeout(
    () => el.classList.remove('v-copied-to-clipboard'),
    500,
  );
});
davidyuk commented 5 years ago

Wouldn't this work the same as your code?

No, it will add an event listener for every emitting of bind, update events, it should somehow remove previously added click listeners.

I think better to add event listener once, and then to update a variable with the current value that should be copied when it necessary. The problem is that directives don't have any own instance and they can only modify the corresponding element. To use dataset in this case is the approach recommended by vue documentation.