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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix-248-Copy-button-issue #565

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/lib/components/copyInput.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
};
</script>

<div>
<div id="tooltip-container">
Copy link
Contributor

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,

console/src/routes/console/project-[project]/settings/updateName.svelte

Lines 48 to 49 in ffb7476

<CopyInput label="Project ID" showLabel={true} value={$project.$id} />
<CopyInput label="API Endpoint" showLabel={true} value={endpoint} />

<label class:u-hide={!showLabel} class="label" for={label}>{label}</label>
<div class="input-text-wrapper" style="--amount-of-buttons:1">
<input {value} id={label} type="text" class="input-text" readonly />
Expand All @@ -37,7 +37,8 @@
on:mouseenter={() => setTimeout(() => (content = 'Click to copy'))}
use:tooltip={{
content,
hideOnClick: false
hideOnClick: false,
appendTo: document.getElementById('tooltip-container')
Copy link
Contributor

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:

Suggested change
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..

}}>
<span class="icon-duplicate" aria-hidden="true" />
</button>
Expand Down