Skip to content

Always return tooltip text, then users could use their own code to fine tune tooltips #20

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

Closed
wants to merge 2 commits into from

Conversation

wxiaoguang
Copy link

@wxiaoguang wxiaoguang commented Mar 19, 2023

Creating a lot of tippy instances is expensive.

I think CalendarHeatmap could always provide the tooltip content by data-tippy-content (even if tooltip=false), then users could use their own code to fine tune tooltips.

Thank you very much!

ps: diff with ignoring space looks clearer 😁

image

@silverwind
Copy link

silverwind commented Mar 19, 2023

Maybe we should give some context, it's go-gitea/gitea#23461 (comment) and my comments below.

@razorness
Copy link
Owner

@silverwind thank you for your hint. @wxiaoguang, I considered to speed up the lib in general at first by moving tooltip generation and tippy initialization towards mouseover events.

As next: I think about making tippy.js optional and add an event (f.e. 'showTooltip') plus the current tooltip method so people can choose whatever they want.

@wxiaoguang
Copy link
Author

wxiaoguang commented Mar 20, 2023

I considered to speed up the lib in general at first by moving tooltip generation and tippy initialization towards mouseover events.

If I understand correctly, tippy uses trigger: 'mouseenter focus', internally.

I have tried an approach and it works:

function lazyTooltipOnMouseEnter(e) {
  e.target.removeEventListener('mouseenter', lazyTooltipOnMouseEnter, true);
  // create the tippy
}

el.addEventListener('mouseenter', lazyTooltipOnMouseEnter, true); // and `focus` event, if necessary.

Using capture will make the lazyTooltipOnMouseEnter execute first, and create the tippy instance, then the tippy instance can handle the current mouseenter event and show.

@razorness
Copy link
Owner

I forgot to mention, that v2.0.2 is just published and you can try yourself.

The magic happens here:

<g class="vch__year__wrapper" :transform="yearWrapperTransform" @mouseover="initTippyLazy">

and here:

function initTippyLazy(e: MouseEvent) {
if (tippySingleton
&& e.target
&& (e.target as HTMLElement).classList.contains('vch__day__square')
&& (e.target as HTMLElement).dataset.weekIndex !== undefined
&& (e.target as HTMLElement).dataset.dayIndex !== undefined
) {
const weekIndex = Number((e.target as HTMLElement).dataset.weekIndex),
dayIndex = Number((e.target as HTMLElement).dataset.dayIndex);
if (!isNaN(weekIndex) && !isNaN(dayIndex)) {
const tooltip = tooltipOptions(heatmap.value.calendar[ weekIndex ][ dayIndex ]);
if (tooltip) {
(e.target as HTMLElement).dataset.tippyContent = tooltip;
if ((e.target as HTMLElement).dataset.tippyInitialized !== 'true') {
(e.target as HTMLElement).dataset.tippyInitialized = 'true';
tippyInstances.push(tippy(e.target as HTMLElement));
tippySingleton.setInstances(tippyInstances);
}
}
}
}
}

If you have a better approach or suggestions, your feedback is welcome! :)

@wxiaoguang
Copy link
Author

Awesome! I think this PR could be closed, thank you very much.

@wxiaoguang wxiaoguang closed this Mar 20, 2023
@razorness
Copy link
Owner

v2.0.3 removes the DOM interaction in favor of a Set to prevent possible issues with Vue rendering.

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.

3 participants