Improve tooltips behavior by kidonng · Pull Request #9668 · refined-github/refined-github

kidonng · 2026-06-01T13:22:03Z

Test URLs

Here

Screenshot

Screen.Recording.2026-06-01.at.9.20.58.PM.mov

kidonng · 2026-06-01T13:32:03Z

+	observe('.rgh-tooltip', tooltip => {
+		tooltip.style.display = '';
+		tooltipContainer!.append(tooltip);
+	});


Reason to #9482 (comment) seems to be that tooltip need to be attached to the DOM after the target element. Hence this dance. I also tried custom elements but that need to be in MAIN world.

kidonng · 2026-06-01T13:33:53Z

😬 Code is not the prettiest but should be most correct

fregante · 2026-06-01T14:02:46Z

So what you're saying is that it works if you append it near the element, and then move it away after the custom element activates?

If so, you can just use a timeout and skip the observer, e.g.

tip = <tool-tip>
el.after(tip)
await delay(0)
ajax.append(tip)

kidonng · 2026-06-01T14:10:36Z

"After" in time, not in location. Timeout does nothing since at this time neither the tooltip nor the target is attached to DOM.

fregante · 2026-06-01T14:16:19Z

What you said contradicts itself. If "after in time", how can "timeout do nothing"? That's what timeout does. If not 0ms then 100ms will do.

I think that in all cases we generate and immediately append the elements, I don't think there's any scenario in which we create the tooltip and append it to the dom at a later time.

fregante · 2026-06-01T14:23:53Z

Actually according to chat you should be able to append them together to the body to "warm them up" and then remove them.

document.body.append(el, tooltip);
el.remove();
tooltip.remove()

at this point you can just add tooltip to the ajaxed container and proceed with the regular element insertion.

kidonng · 2026-06-01T14:27:37Z

Sorry, let me rephrase it more clearly. The tooltip works if attached to the DOM at the same time as the target, or after. It does not work it attached if before target (so we can't container.append(tooltip) then return element).

It may work if we setTimeout(appendTooltip, some_time) and return element, but that leaves entirely to call site whether the target element is appended to DOM immediately after (so tooltip can find its target).

kidonng · 2026-06-01T14:33:46Z

Oh wait it seems to work. I'm still not sure it works all the time though.

fregante

Looks ok as a quick fix

fregante · 2026-06-01T15:36:56Z

 	const tooltip = createTooltipFor(element, content);
 	element.append(tooltip);
+
+	// #9668


Since linkify-code is broken I stopped using this type of comment unless there's a proper comment (and even then). So a full URL is preferred

fregante · 2026-06-01T15:40:01Z

 	margin-left: var(--base-size-6, 0.375rem);
 	overflow: visible;
-	/* eslint-disable-next-line css/font-family-fallbacks */
 	font-family: inherit;


I don't know what's up with this. If Eslint passes with or without this, something must be broken. Restoring...

npm run fix keeps auto-fixing this one. I added it in since it's kinda related...

Yeah it's been annoying. Sorry yeah let's remove it again 😅 and then I'll look into why that's the case separately.

Different plugin, same issue:

eslint --report-unused-disable-directives does not work with this plugin eslint/markdown#115

I'm seeing the error in my IDE as a warning (it should be an error) but $ eslint doesn't even mention it as a warning. So it's not us

fregante · 2026-06-01T15:44:14Z

Does anything from #9508 need to be undone thanks to this fix? I'm thinking the new p- and m- classes added are no longer needed. If you can clean it up in this PR (without affecting sizing/spacing) that'd be great.

This reverts commit dfc7039.

kidonng · 2026-06-01T17:02:22Z

unread-anywhere should be unrelated since it uses addToolTip, not tooltipped

fregante · 2026-06-01T17:04:57Z

#9666 shows this bug on unread-anywhere specifically, so it would be great to target both functions since the issue is the same and this PR wouldn't otherwise close it

fregante · 2026-06-01T17:07:14Z

I think the solution is even easier since element is already attached to the document

const tooltip = createTooltipFor(element, content); element.after(tooltip); // triggers connectedCallback() appendToPjaxContainer(tooltip);

We don't need element.append(tooltip), I kept it in case there's issue appending tooltip to container - a not so perfect tooltip is better than none

Oh I get what you meant earlier now

kidonng · 2026-06-01T17:18:01Z

Right, I fixed addToolTip. But I checked the button styles are still unrelated (and thus needed).

https://github.com/refined-github/refined-github/wiki/Loud-failures

fregante · 2026-06-03T07:22:22Z

 	);

-	// Occasionally this button appears before "Reviewers", so let's wait a bit longer
-	await delay(300);


It turns out that there was one instance in which we waited after generating the element, but this was not necessarily desired behavior.

Fix tooltipped behavior

a643507

kidonng added the bug label Jun 1, 2026

Try #9482 container

9e5ce87

kidonng commented Jun 1, 2026

View reviewed changes

Lint

682a899

create -> setup

bba03cd

kidonng added 3 commits June 1, 2026 22:40

Simplify

a39e435

Ughhh cleanup

f2458fb

Keep original behavior

1d82a0f

kidonng requested a review from fregante June 1, 2026 14:50

fregante approved these changes Jun 1, 2026

View reviewed changes

Discard changes to source/helpers/tooltip.css

dfc7039

fregante changed the title ~~Fix tooltipped behavior~~ Fix tooltips behavior Jun 1, 2026

kidonng added 2 commits June 1, 2026 23:51

Expand comment

773c81d

Revert "Discard changes to source/helpers/tooltip.css"

b7b5a2d

This reverts commit dfc7039.

fregante reviewed Jun 1, 2026

View reviewed changes

Also fix addToolTip

140876c

fregante added 2 commits June 2, 2026 01:29

Comments

f94a508

Loud failures

0c7c2c4

https://github.com/refined-github/refined-github/wiki/Loud-failures

Delay earlier

c7341b4

fregante reviewed Jun 3, 2026

View reviewed changes

fregante changed the title ~~Fix tooltips behavior~~ Improve tooltips behavior Jun 3, 2026

fregante merged commit 6796c84 into main Jun 3, 2026
12 checks passed

fregante deleted the fix/tooltip branch June 3, 2026 07:23

fregante mentioned this pull request Jun 7, 2026

unread-anywhere - Avoid double indicator on profiles #9710

Merged

Uh oh!

Uh oh!

Conversation

kidonng commented Jun 1, 2026

Test URLs

Screenshot

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidonng commented Jun 1, 2026

Uh oh!

fregante commented Jun 1, 2026

Uh oh!

kidonng commented Jun 1, 2026

Uh oh!

fregante commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fregante commented Jun 1, 2026

Uh oh!

kidonng commented Jun 1, 2026

Uh oh!

kidonng commented Jun 1, 2026

Uh oh!

fregante left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fregante commented Jun 1, 2026

Uh oh!

kidonng commented Jun 1, 2026

Uh oh!

fregante commented Jun 1, 2026

Uh oh!

fregante Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidonng commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

fregante commented Jun 1, 2026 •

edited

Loading

fregante Jun 1, 2026 •

edited

Loading

kidonng commented Jun 1, 2026 •

edited

Loading