Allow copying label-value pair to buffer on click #11229

lpessoa · 2022-08-30T03:03:06Z

Addresses #7995
Kept similar DOM structure to keep test compatibility.
Using navigator.clipboard API since it is used by the current standard browsers.
React hot toast is used to notify that the text was successfully copied into clipboard.

roidelapluie · 2022-08-31T13:29:45Z

Thanks for your PR. For legal reasons, we require that all commits are signed with a DCO before we can merge them. See this blog post for considerations around this.

This means that the last line of your commit message should read like:

Signed-Off-By: Your Name <your@email.address>

If you are using GitHub through the web interface, it's quickest to close this PR and open a new one with the appropriate line.

If you are using Git on the command line, it is probably quickest to amend and force push. You can do that with

git commit --amend --signoff
git push -f $remote $remote_branch_for_pr

As always, be careful when force-pushing.

juliusv · 2022-08-31T14:03:01Z

Heya, nice, thanks! A couple of comments:

The Toasted toasts look nice, but since we're already using Bootstrap (via Reactstrap), could we use the Toasts from that to not pull in a new dependency for such a small feature?
A user currently wouldn't know what clicking on a label does, could we add a title="Click to copy label matcher" attribute to the span?
Even with the title, it would be good to visually clarify whether the entire set of matchers is being copied or only one. We could do that by introducing a :hover class for the legen-label-container and setting something like background-color: #add6ff; border-radius: 3px; padding-bottom: 1px; on it:

I'm also thinking it would be good to eventually have a way to copy the entire series identifier, or the metric name. Just being able to copy specifically a single label matcher seems a bit arbitrary to me at least. But I'm not sure how to add that UI-wise for the entire series identifier. I guess we'd have to put a little copy icon next to each series, but that adds a lot of clutter, so let's better sleep on that.

juliusv · 2022-08-31T14:04:28Z

web/ui/react-app/src/pages/graph/SeriesName.tsx

+    navigator.clipboard
+      .writeText(copyText)
+      .then(() => {
+        toast.success('label selector   copied to clipboard', {


Some extra spaces here, and "label matcher" would be the specific technical term for a single label:

Suggested change

toast.success('label selector copied to clipboard', {

toast.success('Label matcher copied to clipboard.', {

Kept similar DOM structure to keep test compatibility. Using `navigator.clipboard` API since it is used by the current standard browsers. React hot toast is used to notify that the text was successfully copied into clipboard. Signed-off-by: lpessoa <luisalmeida@yape.com.pe>

Using the bootstrap toast notification provided by reactstrap. Clipboard handling is managed using React.Context via a shared callback. Updated css according to CR suggestions. Signed-off-by: lpessoa <luisalmeida@yape.com.pe>

lpessoa · 2022-09-03T02:39:32Z

@juliusv added the suggested changes to the code. Maybe i will change the ClipboardContext signature to not only pass the content copied to the clipboard but also the message to be displayed (just to make it a bit more flexible) WDYT?

We can also think of if the user hovers over the { or } we can highlight the whole block in a different color and allow him to copy all labels in one step.

juliusv

Yeah, how to copy the rest of the series components is still an open question IMO, but we don't need to solve it in this PR. The curly braces are too small to comfortably hover over. Maybe hovering over the metric name should select the entire series 🤷‍♂️

juliusv · 2022-09-05T13:52:32Z

web/ui/react-app/src/pages/graph/SeriesName.tsx

@@ -16,7 +34,7 @@ const SeriesName: FC<SeriesNameProps> = ({ labels, format }) => {
      }

      labelNodes.push(
-        <span key={label}>
+        <span className="legend-label-container" onClick={toClipboard} key={label} title="Click to copy label matcher">


I noticed that the hovering now includes the comma between labels as well.

How about changing the HTML here so that the legend-label-container only wraps the k=v part without the comma, like:

 {!first && ', '} {label}="{labels[label]}" 

Then you can also drop the logic around the comma in toClipboard.

Thought of that too. I have to confess that i did not go with that approach since it would require to change the existing tests a little bit 😄... but it is the proper way to go.

juliusv · 2022-09-05T13:56:12Z

web/ui/react-app/src/contexts/ClipBoardContext.tsx

@@ -0,0 +1,11 @@
+import React from 'react';
+
+const ClipboardContext = React.createContext((msg: string) => {


I would tend to not call this ClipboardContext, but just ToastContext, to both clarify what it's actually about and make it more general in use. When I first read the name, I assumed this was for storing the clipboard message (but I think keeping the clipboard message just in the actual clipboard as you do it now is good).

lpessoa · 2022-09-12T00:52:57Z

@juliusv applied suggested changes and reworked the tests 😃

juliusv · 2022-09-12T05:17:41Z

Thanks, looks good now, just the DCO CI check is failing now with this message for one of the commits:

Commit sha: dd254c8, Author: Luis Pessoa, Committer: Luis Pessoa; Expected "Luis Pessoa luisalmeida@yape.com.pe", but got "lpessoa luisalmeida@yape.com.pe".

I guess it expects you to sign off with your full name that you also use as the author in Git.

Cleaning up renderFormatted method. Renamed Clipboard to ToastContext. Updated tests. Signed-off-by: Luis Pessoa <luisalmeida@yape.com.pe>

lpessoa · 2022-09-12T14:32:08Z

Done... no idea how it got lpessoa instead of Luis Pessoa 🤷🏼‍♂️ @juliusv

juliusv · 2022-09-20T12:30:02Z

Ah, belated thanks, gonna merge 👍

lpessoa requested a review from juliusv as a code owner August 30, 2022 03:03

juliusv reviewed Aug 31, 2022

View reviewed changes

lpessoa force-pushed the label-clipboard branch from c681a6d to cf3d3e1 Compare August 31, 2022 18:01

Using reactstrap for toast notification

cc01c11

Using the bootstrap toast notification provided by reactstrap. Clipboard handling is managed using React.Context via a shared callback. Updated css according to CR suggestions. Signed-off-by: lpessoa <luisalmeida@yape.com.pe>

lpessoa force-pushed the label-clipboard branch from c1d13f3 to cc01c11 Compare September 3, 2022 02:29

juliusv reviewed Sep 5, 2022

View reviewed changes

Changes from CR comments

4a9c2b5

Cleaning up renderFormatted method. Renamed Clipboard to ToastContext. Updated tests. Signed-off-by: Luis Pessoa <luisalmeida@yape.com.pe>

lpessoa force-pushed the label-clipboard branch from dd254c8 to 4a9c2b5 Compare September 12, 2022 14:30

juliusv merged commit 9591103 into prometheus:main Sep 20, 2022

lpessoa deleted the label-clipboard branch November 19, 2023 17:03

beorn7 mentioned this pull request May 21, 2024

Add labels to Selector by clicking on labels in result set [React UI] #7995

Closed

vitoyucepi mentioned this pull request Jul 7, 2024

navigator.clipboard may not be available #14432

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow copying label-value pair to buffer on click #11229

Allow copying label-value pair to buffer on click #11229

lpessoa commented Aug 30, 2022

roidelapluie commented Aug 31, 2022

juliusv commented Aug 31, 2022

juliusv Aug 31, 2022

lpessoa commented Sep 3, 2022 •

edited

Loading

juliusv left a comment

juliusv Sep 5, 2022

lpessoa Sep 5, 2022

juliusv Sep 5, 2022

lpessoa commented Sep 12, 2022

juliusv commented Sep 12, 2022

lpessoa commented Sep 12, 2022 •

edited

Loading

juliusv commented Sep 20, 2022

	toast.success('label selector copied to clipboard', {
	toast.success('Label matcher copied to clipboard.', {

		@@ -0,0 +1,11 @@
		import React from 'react';

		const ClipboardContext = React.createContext((msg: string) => {

Allow copying label-value pair to buffer on click #11229

Allow copying label-value pair to buffer on click #11229

Conversation

lpessoa commented Aug 30, 2022

roidelapluie commented Aug 31, 2022

juliusv commented Aug 31, 2022

juliusv Aug 31, 2022

Choose a reason for hiding this comment

lpessoa commented Sep 3, 2022 • edited Loading

juliusv left a comment

Choose a reason for hiding this comment

juliusv Sep 5, 2022

Choose a reason for hiding this comment

lpessoa Sep 5, 2022

Choose a reason for hiding this comment

juliusv Sep 5, 2022

Choose a reason for hiding this comment

lpessoa commented Sep 12, 2022

juliusv commented Sep 12, 2022

lpessoa commented Sep 12, 2022 • edited Loading

juliusv commented Sep 20, 2022

lpessoa commented Sep 3, 2022 •

edited

Loading

lpessoa commented Sep 12, 2022 •

edited

Loading