-
Notifications
You must be signed in to change notification settings - Fork 147
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
Fix-248-Copy-button-issue #565
Conversation
Someone is attempting to deploy a commit to the appwrite Team on Vercel. A member of the Team first needs to authorize it. |
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.
Great PR! 🤯 We left some comments during the review, please check them out.
@@ -24,7 +24,7 @@ | |||
}; | |||
</script> | |||
|
|||
<div> | |||
<div id="tooltip-container"> |
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 best to avoid IDs when possible since they're highly selective regarding CSS, and IDs should be unique, but this component can be used multiple times/places. For example,
<CopyInput label="Project ID" showLabel={true} value={$project.$id} /> | |
<CopyInput label="API Endpoint" showLabel={true} value={endpoint} /> |
@@ -37,7 +37,8 @@ | |||
on:mouseenter={() => setTimeout(() => (content = 'Click to copy'))} | |||
use:tooltip={{ | |||
content, | |||
hideOnClick: false | |||
hideOnClick: false, | |||
appendTo: document.getElementById('tooltip-container') |
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.
Nice find! I do think this is on the right track. Looking at the documentation, appendTo can be a function that takes a reference to the element itself, so we can change this to:
appendTo: document.getElementById('tooltip-container') | |
appendTo: (ref) => ref.parentElement.parentElement |
Be sure to add a comment or commit message to explain why we need to do ref.parentElement.parentElement
..
What does this PR do?
Shows Copy pop while hovering on the copy button and shows 'copied' when button is clicked
Test Plan
Tested locally
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
(Write your answer here.)