fix(headings): copies heading link to clipboard#4805
fix(headings): copies heading link to clipboard#4805kmcfaul merged 4 commits intopatternfly:mainfrom
Conversation
srambach
requested review from
a team,
dlabaj,
edonehoo,
mcoker and
rebeccaalpert
and removed request for
a team
rebeccaalpert
left a comment
rebeccaalpert
left a comment
There was a problem hiding this comment.
Copying looks fine, but the description of the ticket doesn't mention dropping the link functionality (at least to me). Do we want to maintain it also? This PR drops it completely.
Consensus seems to be it doesn't make too much sense to jump, since you are already there. Other design systems seem to jump or copy the link but not both. The only use case we could think of is to scroll to an exact position for comparison across browser tabs, which seems like a pretty niche use case. I'm going to leave it off for now and if requested we can consider adding it in the future. |
|
this looks great! It looks like the tooltip goes back to the hover message before it dismisses. Idk if that's just me or just a quirk to put up with, but wanted to mention Screen.Recording.2025-09-29.at.4.40.05.PM.mov |
| import React from 'react'; | ||
| import { Flex, FlexItem, Content } from '@patternfly/react-core'; | ||
| import React, { useState } from 'react'; | ||
| import { Flex, FlexItem, Content, Tooltip, Button } from '@patternfly/react-core'; |
There was a problem hiding this comment.
Doesn't look like <Button> is being used anywhere. Though the styles are pretty similar to the no-padding button, you could use it below. No padding button increases the click area with a negative inset via --#{$button}--m-plain--m-no-padding--border--offset if you want to match it what's in this PR
| console.log('URL copied to clipboard:', url); | ||
| }).catch(err => { | ||
| console.error('Failed to copy URL:', err); |
There was a problem hiding this comment.
Probably want to take the console.logs out?
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
Nice! Left a couple of comments, but also when you hover/focus the button it says "copy link to clipboard," then says "link copied to clipboard" when you click on it, and if you don't click on anything else, the tooltip changes back to "copy link to clipboard" before disappearing. You may look into the code editor header under our examples to see how it handles the popover because it doesn't do that. It goes from "copy code to clipboard" to "code copied", then just disappears. You could also match that wording - "copy link to clipboard" and "link copied"
dlabaj
left a comment
dlabaj
left a comment
There was a problem hiding this comment.
Other then cokers comments LGTM.
7 participants
Fixes #4370
This changes the link icon next to the headings to copy the link to the clipboard (and show a tooltip indicating this) rather than linking to the anchor.