-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
copy to clipboard feature added #2551
copy to clipboard feature added #2551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great having you contribute to this project
Welcome to the community 🤓If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.
components/ClipboardCopy.js
Outdated
}, 1500); | ||
}) | ||
.catch((err) => { | ||
console.log(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using async/await and try/catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh okay, I'll fix that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you 👍
components/ClipboardCopy.js
Outdated
@@ -0,0 +1,51 @@ | |||
import React, { useState } from "react"; | |||
|
|||
const ClipboardCopy = ({ copyText, children }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need copyText
and children
? They look the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The children
have the code blocks commas in the start and end. Should I use slice to remove the firsts 5 and the last three characters? Then We could probably use the children
prop itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good point 🤔 I think yes, because I am keen to try and use the same code, so there is not duplicate and not the risk of the code being different and causing bugs for people.
It would probably be safer to separate on the line breaks and remove the first line and last line - because it might not always be 5 characters at the beginning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good point 🤔 I think yes, because I am keen to try and use the same code, so there is not duplicate and not the risk of the code being different and causing bugs for people.
It would probably be safer to separate on the line breaks and remove the first line and last line - because it might not always be 5 characters at the beginning
Oh yeah, That is a way better option. Could you tell me how would I convert the children
prop to string. I can't seem to find the method to get the text from the object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tried before, but after a Google I found this https://github.com/fernandopasik/react-children-utilities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give it a go, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I left some inline questions
Hey Eddie! I fixed it 🕺 |
Looks good @ChinmayMhatre 👍 I will have a play with it later on and then we can get it merged Thank you! |
Awesome! Maybe later on we could move the wrapper component to a utils folder. Could create an issue later on, a green square for a first timer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍
Fixes Issue
Closes #2521
Changes proposed
Added the copy to clipboard button to the code blocks in the quick start page.
https://linkfree.eddiehub.io/docs/quickstart
Check List (Check all the applicable boxes)
Screenshots