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

copy button added #5524

Closed
wants to merge 14 commits into from
Closed

Conversation

narasimha-1511
Copy link
Member

@narasimha-1511 narasimha-1511 commented May 6, 2024

Description

This PR fixes #5517

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Narasimha <129654598+narasimha-1511@users.noreply.github.com>
@l5io
Copy link
Contributor

l5io commented May 6, 2024

🚀 Preview for commit d3e9ae5 at: https://66386ba8f6a8fe76cda63d22--layer5.netlify.app

@narasimha-1511
Copy link
Member Author

Updated With the Button

image

@vishalvivekm
Copy link
Member

vishalvivekm commented May 6, 2024

Adding as an agenda item for websites call.
https://docs.google.com/document/d/1XczAHXVe2FIWPqiF57ospJ43zw5cZQ7ui8mn39v5EvA/edit

@Rexford74 Rexford74 requested review from lakshz and Rexford74 May 6, 2024 12:21
@lakshz
Copy link
Contributor

lakshz commented May 6, 2024

@Rexford74 Should we use a copy icon in place of Copy text. And, a success tick icon in place Copied! text.

@Rexford74
Copy link

Those are fine suggestions @lakshz and I think they can actually fly.
Also, if you check out the design, you will see that this has already been accounted for. And I think something that was missing in your initial implementation @lakshz is the addition of the code sandbox, restart, and more icons in the same row with the 'Show Code' checkbox. Take a look at the prototype so that you can get a glimpse of the desired behaviour.
@narasimha-1511 and @lakshz please you are welcome to offer your thoughts on the issues that I just raised. If it is possible to have it anything like in the current implementation, please let me know. If not, you can offer your thoughts so that we can find a solution to any blockers you might have in rounding up this issue as soon as possible.

@lakshz
Copy link
Contributor

lakshz commented May 8, 2024

@Rexford74 Got it. The "Show code" section needs more features, in addition to the Copy button.
However, I would clear some things from the prototype:

Screenshot 2024-05-08 at 22 18 25
  • The code sandbox should be dynamic or static? I mean, is the code editable, and should the above buttons change based on the below code?

  • Do we need 3 buttons together in one sandbox? Or is it just for reference?

@narasimha-1511
Copy link
Member Author

@lakshz I think 3 buttons are not Nedded !

@lakshz
Copy link
Contributor

lakshz commented May 12, 2024

@narasimha-1511 Let's stick to adding a copy button in this PR. We can re-iterate on the other features (if they are needed) moving forward.

I've left some comments, for you to solve.

Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@ritiksaxena124
Copy link
Contributor

Hey @narasimha-1511 , let's discuss this on today's website meeting at 12:00 pm GMT / 5:30 PM IST. I am adding this as an agenda item. Please join if you are available during the meet.

narasimha-1511 and others added 3 commits May 13, 2024 07:37
Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
All changes done as requested
@l5io
Copy link
Contributor

l5io commented May 13, 2024

🚀 Preview for commit 9b66ec2 at: https://6641c7241107b276d1a0c159--layer5.netlify.app

narasimha-1511 and others added 2 commits May 23, 2024 06:39
Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
importing the color into the svg
@l5io
Copy link
Contributor

l5io commented May 23, 2024

🚀 Preview for commit 775d278 at: https://664ee88be3d958cbacaf54f7--layer5.netlify.app

Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@l5io
Copy link
Contributor

l5io commented May 23, 2024

🚀 Preview for commit 3ae961e at: https://664ef04482a7a5c1e72c673b--layer5.netlify.app

@sudhanshutech
Copy link
Member

@narasimha-1511 whats the status here, will you share your progress in upcoming website meet?

@narasimha-1511
Copy link
Member Author

Yeah for sure @sudhanshutech

@leecalcote
Copy link
Member

@narasimha-1511 what is that status?

@narasimha-1511
Copy link
Member Author

@leecalcote Almost Done , Need a Review

@narasimha-1511
Copy link
Member Author

narasimha-1511 commented May 30, 2024

Recording.2024-05-30.155053.mp4

@leecalcote
Copy link
Member

@lakshz we're ready for a final round of review here.

@lakshz
Copy link
Contributor

lakshz commented Jun 7, 2024

@narasimha-1511 Instead of showing a toast on copying the code, replace the copy icon with a success tick icon.
This is because we are not using react-toastify anywhere before.

@narasimha-1511
Copy link
Member Author

@lakshz Sure ! , i will make the following changes

@Rexford74
Copy link

@Rexford74 can you provide him the correct svg
Which SVG are you referring to? The copy icon, or is there another one that you have in mind?

@narasimha-1511
Copy link
Member Author

Hey @sudhanshutech ,
i have made all the changes
i will commit them if i can get the correct svg
@Rexford74

@narasimha-1511
Copy link
Member Author

narasimha-1511 commented Jul 1, 2024

@Rexford74 Got it. The "Show code" section needs more features, in addition to the Copy button. However, I would clear some things from the prototype:

Screenshot 2024-05-08 at 22 18 25 * The code sandbox should be dynamic or static? I mean, is the code editable, and should the above buttons change based on the below code? * Do we need 3 buttons together in one sandbox? Or is it just for reference?

Hey @sudhanshutech i have taken the copy svg from the prototype here!

Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@Rexford74
Copy link

Copy Icon
I'm guessing this is the copy icon that you mean. Let me know if you'd need anything else.

@sudhanshutech
Copy link
Member

@narasimha-1511 any progress? Did you update the changes as we reviewed in call?

@narasimha-1511
Copy link
Member Author

@sudhanshutech The changes have been made! Apologies for the delay.

Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@l5io
Copy link
Contributor

l5io commented Jul 6, 2024

🚀 Preview for commit 6b22c78 at: https://66892d26445f1d4074a148e1--layer5.netlify.app

@sudhanshutech
Copy link
Member

sudhanshutech commented Jul 11, 2024

@narasimha-1511 follow this design to add the copy button , design can be found here: figma)
image

@narasimha-1511
Copy link
Member Author

narasimha-1511 commented Jul 11, 2024

Hey @sudhanshutech I have taken the copy icon from the prototype itself that you have given

@narasimha-1511
Copy link
Member Author

@sudhanshutech
Copy link
Member

sudhanshutech commented Jul 11, 2024

its opening fine for me. You can directly go through layer5 figma and under project sistent

Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@l5io
Copy link
Contributor

l5io commented Jul 11, 2024

🚀 Preview for commit 2a8895e at: https://668f7b61a59060f07d227a5f--layer5.netlify.app

@sudhanshutech
Copy link
Member

sudhanshutech commented Jul 11, 2024

@narasimha-1511 please follow these points.

  1. Directly import copy icon from sistent , you will get that icon inside sistent repo. Check which icon is the suitable, if i am remeber it should be with name cloneicon. Check this
    To be specific:
import <Icon> from '@layer5/sistent`
  1. Place the copy where its been design in figma, a screenshot i already sent you above and given the figma link.

@narasimha-1511
Copy link
Member Author

@sudhanshutech i have done what you said one last doubt

image

in the image i have placed the icon at the green spot do you want me to place it at the red spot also as you have given in the screenshot?

image

also the icon i used from the layer5/sisent the Cloneicon as you said is not lookig good i have tried to set the height and width still i think the icon is not same as in the figma provided

@narasimha-1511
Copy link
Member Author

@sudhanshutech i have done what you said one last doubt

image

in the image i have placed the icon at the green spot do you want me to place it at the red spot also as you have given in the screenshot?

image

also the icon i used from the layer5/sisent the Cloneicon as you said is not lookig good i have tried to set the height and width still i think the icon is not same as in the figma provided

Hey @sudhanshutech please review this

@leecalcote
Copy link
Member

Merge conflict.

narasimha-1511 and others added 2 commits July 21, 2024 15:49
Signed-off-by: Narasimha <129654598+narasimha-1511@users.noreply.github.com>
Signed-off-by: Narasimha <s.narasimha.2005@gmail.com>
@l5io
Copy link
Contributor

l5io commented Jul 21, 2024

🚀 Preview for commit 2cd0ff5 at: https://669ce81ae2b43959e5078281--layer5.netlify.app

@narasimha-1511
Copy link
Member Author

Megre Conflict Resolved @leecalcote @sudhanshutech @Ashparshp

@Ashparshp
Copy link
Contributor

@narasimha-1511 Thanks for the update!

@sudhanshutech
Copy link
Member

@narasimha-1511 this copy will not be needed now because the code block is updated to use by default copy functionality. Please do take a look

@narasimha-1511
Copy link
Member Author

sure @sudhanshutech
I think the copy button that is present now needs to be updated to a icon
also there is a gap betwwen the show code and the code block?
image

@sudhanshutech
Copy link
Member

Make sense. Please do create issue for those.

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

Successfully merging this pull request may close these issues.

[Sistent] Add a copy button to copy code
9 participants