Skip to content
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

feature: copyable field option #3468

Merged
merged 28 commits into from
Dec 20, 2024
Merged

Conversation

Nevelito
Copy link
Contributor

@Nevelito Nevelito commented Nov 28, 2024

Description

The copyable option enables users to copy the field's value to their clipboard. When set to true, a clipboard icon appears when hovering over the field value, allowing easy copying. This feature can be particularly useful for fields such as unique identifiers, URLs, or other text-based content that users may frequently need to copy.

Fixes #2769

copyable.mp4

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Copy link

codeclimate bot commented Nov 28, 2024

Code Climate has analyzed commit ecab4d5 and detected 0 issues on this pull request.

View more on Code Climate.

@KamilMilewski
Copy link
Contributor

We changed quite a lot in existing components or added new ones. Should we cover those changes with specs?

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all your effort here, @Nevelito!

I've been testing it locally, and it's already working as expected.

Let's make some visual adjustments and apply the same design used in the Docs (check it here).

Specifically:

  • Display the empty clipboard on hover, without the text.
  • After a click, display the same clipboard with a checkmark inside it.

Later, we could add a title to the icon and display it on hover.

I've also included some comments about the code.

Let me know if you have any questions.

spec/dummy/db/schema.rb Outdated Show resolved Hide resolved
app/components/avo/fields/show_component.rb Outdated Show resolved Hide resolved
@Paul-Bob
Copy link
Contributor

Paul-Bob commented Dec 9, 2024

We changed quite a lot in existing components or added new ones. Should we cover those changes with specs?

Let's do a system spec at the end that tests the copyable functionality, not component specific.

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the configurations that https://www.stimulus-components.com/docs/stimulus-clipboard already supports

Comment on lines 15 to 18
setTimeout(() => {
this.iconTarget.classList.remove('hidden')
this.iconCopiedTarget.classList.add('hidden')
}, 2000)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be configured with data-clipboard-success-duration-value

From: https://www.stimulus-components.com/docs/stimulus-clipboard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not that easy, It works with data-clipboard-success-content-value (this work only with plain text) , we want to show another object so we can not probably use data-clipboard-success-duration-value. I tried use that but it do not work.

@Nevelito Nevelito requested a review from Paul-Bob December 13, 2024 12:17
@Paul-Bob Paul-Bob changed the title Feature/click to copy feature: copyable field option Dec 16, 2024
Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution @Nevelito it's looking great!

I just pushed some commits to complete the PR.

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Dec 17, 2024

TODO:

  • add tool-tip
  • add localization key for copy
  • Docs

@Paul-Bob Paul-Bob merged commit cbad870 into avo-hq:main Dec 20, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Click to copy field option
3 participants