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.

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

IDE-style tooltips #209

Merged
merged 12 commits into from
May 10, 2024
Merged

IDE-style tooltips #209

merged 12 commits into from
May 10, 2024

Conversation

tayloraswift
Copy link
Owner

my thinking around tooltips is we eventually want to support them everywhere, not just in codelinks. so we should not try to emit tooltips inline in the HTML renderer because

  • difficult to style prose that is lexically-embedded inside a <code> element
  • difficult to reuse code to add tooltips to other things besides codelinks
  • won’t work with href inlining, because we cannot inject HTML elements into an HTML attribute, which would basically mean no IndexstoreDB tooltips

instead, we will attach pre-rendered tooltip elements to some hidden section of HTML, and it will then be some client-side javascript’s responsibility to attach the tooltips to any element that has a valid href attribute

pros:

  • avoids re-transmitting the same tooltip HTML if multiple links to the same symbol exist on the same page
  • less complexity in the renderer
  • can theoretically attach tooltips to anything that links to a symbol

cons:

  • no way to associate tooltips with symbols, except by URL, and URLs can be very, very long
  • involves javascript, which means render API clients will also need to provide custom javascript alongside the custom CSS we are already asking of them

@tayloraswift
Copy link
Owner Author

cc @Joannis

@tayloraswift tayloraswift self-assigned this May 6, 2024
@Joannis
Copy link
Contributor

Joannis commented May 7, 2024

Sounds like a good plan to me!

@tayloraswift
Copy link
Owner Author

so, as a POC, this is working, but there are some weird frontend issues that need to be worked out before this is production ready.

right now we are rendering the tooltips on the client-side in the inline “analog” way

  • client-side JS embeds the tooltip HTML as DOM children of the <a> elements
  • CSS handles the visibility logic

pros:

  • can leverage CSS :hover, etc. to make the tooltips appear and disappear
  • tooltips automatically appear next to the associated <a> element, no extra positioning logic needed
  • simpler to implement on the Typescript side

cons:

  • tooltips are not responsive to the layout of the page - tooltips on anchors all the way to the right of the screen will flow even further right
  • hard to style tooltips correctly
  • there are a lot of weird, janky CLS issues with tooltips that are nominally part of the document flow. e.g, hovering over a link with a tooltip changes the line breaking behavior of the paragraph containing the link.

i think embedding tooltips inside the <a> elements is the wrong approach; the only reason we are doing this is to be able to position the tooltip relative to the location of the <a> element on the screen. but we would need additional positioning logic anyways, to make the tooltips responsive to the user’s screen

@tayloraswift
Copy link
Owner Author

my gut feeling is that the tooltips really need to be a separate layer, disconnected from the HTML of the underlying article. e.g, they should be position: fixed; and the JS should compute the correct (x, y) positions like those Modern Web Frameworks do. to do this, we probably need to roll our own hover events logic in JS

@tayloraswift
Copy link
Owner Author

positioning the tooltips dynamically from JS was definitely the way to go, it was a lot easier than i imagined

@tayloraswift
Copy link
Owner Author

for render API users, this appends a hidden <div> node to the end of the rendered HTML fragment with id='ss:tooltips'. to enable the tooltips on the external site requires some javascript resembling

if (tooltips !== null) {
tooltips.remove();
// The tooltips `<div>` contains `<a>` elements only.
let cards: { [id: string] : HTMLSpanElement; } = {};
let frame: HTMLDivElement = document.createElement('div');
for (const anchor of tooltips.children) {
if (!(anchor instanceof HTMLAnchorElement)) {
continue;
}
// Cannot use `anchor.href`, we want the exact value of the `href` attribute.
const id: string | null = anchor.getAttribute("href")
if (id === null) {
continue;
}
// Change the tooltip into a `<div>` with `class="tooltip"`.
const tooltip: HTMLSpanElement = document.createElement('div');
tooltip.innerHTML = anchor.innerHTML;
cards[id] = tooltip;
frame.appendChild(tooltip);
}
// Inject the tooltips into every `<a>` element with the same `href` attribute.
// This should only be done within the `<main>` element.
const main: HTMLElement | null = document.querySelector('main');
if (main !== null) {
main.querySelectorAll('a').forEach((
anchor: HTMLAnchorElement,
key: number,
all: NodeListOf<Element>
) => {
// If the anchor is inside a card preview, the tooltip would be redundant.
if (anchor.parentElement?.tagName === 'CODE' &&
anchor.parentElement.classList.contains('decl')) {
return;
}
if (anchor.parentElement?.tagName === 'H3' &&
anchor.parentElement.classList.contains('module')) {
return;
}
const id: string | null = anchor.getAttribute("href")
if (id === null) {
return;
}
const tooltip: HTMLSpanElement | undefined = cards[id];
if (tooltip === undefined) {
return;
}
// When you hover over the anchor, show the tooltip by loading the (x, y) position
// of the anchor on the screen, and then adding the tooltip to the document as
// a fixed-position element.
anchor.addEventListener('mouseenter', (event: MouseEvent) => {
const r: DOMRect = anchor.getBoundingClientRect();
tooltip.style.left = r.x.toString() + 'px';
tooltip.style.top = r.bottom.toString() + 'px';
tooltip.classList.add('visible');
});
anchor.addEventListener('mouseleave', (event: MouseEvent) => {
tooltip.classList.remove('visible');
});
});
// Make the tooltips list visible; it was originally hidden to prevent FOUC.
frame.className = 'tooltips';
document.body.appendChild(frame);
}
}

@tayloraswift tayloraswift marked this pull request as ready for review May 10, 2024 23:44
@tayloraswift
Copy link
Owner Author

linking these two issues which are not blocking this PR but are far more noticeable with the new feature:

@tayloraswift tayloraswift merged commit c45f779 into master May 10, 2024
2 checks passed
@tayloraswift tayloraswift deleted the tooltips branch May 10, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants