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

[Add] Copy to Clipboard Button #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ihsonnet
Copy link

Copy to Clipboard button has been added to make it easier for users to copy text.

@biswajit-saha
Copy link
Owner

Hi,
Thanks for the PR. But I think there are some issue In my opinion. Some are best practice and some are UX issue.
Which you should consider to change.

The Biggest issue is copy is not working always. Do the following to test it.

  1. Generate text for the first time.
  2. Copy it by clicking on the copy button.
  3. Click outside of the content area (to deselect the text )
  4. Generate text again.
  5. Click button to copy the text again. The new text is not being selected and copied.

I have used Gulp in the project so please consider to use that in your work flow.

If you install the dependency and then run gulp command all .scss files and main.js file will be compiled and minified and a local server will be running.

Now my suggested changes.

  • Do not use any inline css within HTML file.
  • Do not add extra ID to html element just for styling. Use existing class or add new one if needed.
  • Move all your extra css in scss.
  • Move your javacript in main.js ( this file will be minified by gulp)
  • If you do above two things then you don't have to load load main.js, copy.js, tooltip.css and screen.css.
  • Also please note, the minified version of screen.css is already being loaded in the HTML file. And whenever we build the site using Gulp screen.css and screen.min.css will be regenerated. So any changes in either of this file will be lost.
    So the best practice is using SCSS.

UX issue

  • The copy text button is visible when site loads at the first time. But there is no text to copy. Show the button only after text generation done.
  • Copy button it too wide, but not the same height as other button or input field. Also font size is not matched. Try to make it same.

@ihsonnet
Copy link
Author

oky sir, thanks for your suggestion, i'll try to fixed them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants