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

Tooltips fixes #12067

Merged
merged 8 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [Quick Fix Import Button][12051].
- [Fixed nodes being selected after deleting other nodes or connections.][11902]
- [Redo stack is no longer lost when interacting with text literals][11908].
- [Tooltips are hidden when clicking on a button][12067].
- [Fixed bug when clicking header in Table Editor Widget didn't start editing
it][12064]
- [Fixed bugs occurring after renaming project from within graph editor][12106].
Expand All @@ -18,6 +19,7 @@
[12051]: https://github.com/enso-org/enso/pull/12051
[11902]: https://github.com/enso-org/enso/pull/11902
[11908]: https://github.com/enso-org/enso/pull/11908
[12067]: https://github.com/enso-org/enso/pull/12067
[12064]: https://github.com/enso-org/enso/pull/12064
[12106]: https://github.com/enso-org/enso/pull/12106

Expand Down
8 changes: 7 additions & 1 deletion app/gui/src/project-view/components/MenuButton.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<script setup lang="ts">
import TooltipTrigger from '@/components/TooltipTrigger.vue'
import { ref } from 'vue'
import type { ComponentExposed } from 'vue-component-type-helpers'

/**
* A button. Supports toggling and disabled state.
Expand All @@ -13,14 +15,18 @@ import TooltipTrigger from '@/components/TooltipTrigger.vue'

const toggledOn = defineModel<boolean>({ default: undefined })
const props = defineProps<{ disabled?: boolean | undefined; title?: string | undefined }>()
const tooltipTrigger = ref<ComponentExposed<typeof TooltipTrigger>>()

function onClick() {
if (!props.disabled && toggledOn.value != null) toggledOn.value = !toggledOn.value
if (tooltipTrigger.value) {
tooltipTrigger.value.hideTooltip()
}
}
</script>

<template>
<TooltipTrigger>
<TooltipTrigger ref="tooltipTrigger">
<template #default="triggerProps">
<button
class="MenuButton clickable"
Expand Down
100 changes: 80 additions & 20 deletions app/gui/src/project-view/components/TooltipDisplayer.vue
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<script setup lang="ts">
import { type TooltipRegistry } from '@/providers/tooltipRegistry'
import { debouncedGetter } from '@/util/reactivity'
import { autoUpdate, flip, offset, shift, useFloating } from '@floating-ui/vue'
import { computed, ref } from 'vue'
import { HoveredElement, type TooltipRegistry } from '@/providers/tooltipRegistry'
import { autoUpdate, flip, FloatingElement, offset, shift, useFloating } from '@floating-ui/vue'
import { computed, ref, shallowRef, toValue, watch } from 'vue'

const props = defineProps<{ registry: TooltipRegistry }>()

Expand All @@ -11,37 +10,98 @@ const TOOLTIP_SHOW_DELAY_MS = 1500
// Time after which tooltip will disappear once an element is no longer hovered.
const TOOLTIP_HIDE_DELAY_MS = 300

const activeTooltip = computed(() => {
const element = props.registry.lastHoveredElement.value
return { element, entry: props.registry.getElementEntry(element) }
// Currently hovered element and its tooltip entry.
const activeTooltip = props.registry.lastHoveredElement
// Previously hovered element and its tooltip entry.
const previousTooltip = shallowRef<HoveredElement>()
const show = ref(false)

type Timeout = ReturnType<typeof setTimeout> | null
let hideTimeout: Timeout
let showTimeout: Timeout

function resetTimeout(timeout: Timeout) {
if (timeout != null) {
clearTimeout(timeout)
timeout = null
}
}

watch(activeTooltip, (newValue, oldValue) => {
if (oldValue == null && newValue != null) {
// We are hovering some element, show its tooltip after delay.
resetTimeout(showTimeout)
resetTimeout(hideTimeout)
showTimeout = setTimeout(() => {
show.value = true
showTimeout = null
}, TOOLTIP_SHOW_DELAY_MS)
previousTooltip.value = toValue(newValue)
} else if (oldValue != null && newValue == null) {
// We no longer hover any element, hide the tooltip after delay.
resetTimeout(showTimeout)
resetTimeout(hideTimeout)
hideTimeout = setTimeout(() => {
show.value = false
hideTimeout = null
}, TOOLTIP_HIDE_DELAY_MS)
}
// There is no need to check if both `oldValue` and `newValue` are not null, because
// `activeTooltip` is always set to null intermitently when switching between elements.
})
const doShow = debouncedGetter(
() => activeTooltip.value.element != null && activeTooltip.value.entry != null,
TOOLTIP_SHOW_DELAY_MS,
)
const displayedEntry = debouncedGetter(activeTooltip, TOOLTIP_HIDE_DELAY_MS)
const floatTarget = computed(() => {
return doShow.value && displayedEntry.value.element?.isConnected ?
displayedEntry.value.element
: undefined

const displayedTooltip = computed(() => {
if (!show.value) return undefined
// When hovering the element, display its tooltip.
if (
activeTooltip.value != null &&
!activeTooltip.value.entry.isHidden &&
activeTooltip.value.element.isConnected
) {
return activeTooltip.value
}
// If no element with the tooltip is being hovered, display the tooltip of the previously hovered element.
// (until it will be hidden after timeout, by changing the `show` ref)
if (
previousTooltip.value != null &&
!previousTooltip.value.entry.isHidden &&
previousTooltip.value.element.isConnected
) {
return previousTooltip.value
}
Comment on lines +65 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining why we sometimes display the tooltip at the previous element - I think it's not obvious without the context of the bug you're fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return undefined
})
const floatTarget = computed(() => displayedTooltip.value?.element)
const tooltipContents = computed(() => displayedTooltip.value?.entry.contents.value)

// Default implementation of autoUpdate ignores cases when the reference is being unmounted while in animation.
// This is a hotfix for that.
function whileElementsMounted(
reference: HTMLElement,
floating: FloatingElement,
update: () => void,
): () => void {
return autoUpdate(reference, floating, () => {
if (reference.isConnected) {
update()
}
})
}

const tooltip = ref<HTMLDivElement>()
const floating = useFloating(floatTarget, tooltip, {
placement: 'top',
transform: false,
middleware: [offset(5), flip(), shift()],
whileElementsMounted: autoUpdate,
whileElementsMounted,
})

const tooltipContents = computed(() => displayedEntry.value.entry?.contents.value)
</script>

<template>
<Transition>
<div
v-if="floatTarget != null && tooltipContents != null"
:key="displayedEntry.entry?.key ?? 0"
:key="displayedTooltip?.entry.key ?? 0"
ref="tooltip"
class="Tooltip"
:style="floating.floatingStyles.value"
Expand Down
10 changes: 8 additions & 2 deletions app/gui/src/project-view/components/TooltipTrigger.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,22 @@ const slots = defineSlots<{
const tooltipSlot = toRef(slots, 'tooltip')
const registered = registry.registerTooltip(tooltipSlot)
function onEnter(e: PointerEvent) {
if (e.target instanceof HTMLElement) {
if (e.target instanceof HTMLElement && tooltipSlot.value != null) {
registered.onTargetEnter(e.target)
}
}

function onLeave(e: PointerEvent) {
if (e.target instanceof HTMLElement) {
if (e.target instanceof HTMLElement && tooltipSlot.value != null) {
registered.onTargetLeave(e.target)
}
}

defineExpose({
hideTooltip() {
registered.forceHide()
},
})
</script>

<template>
Expand Down
58 changes: 46 additions & 12 deletions app/gui/src/project-view/providers/tooltipRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,47 @@ import {
type ShallowReactive,
type Slot,
} from 'vue'
import { assert } from 'ydoc-shared/util/assert'

interface TooltipEntry {
contents: Ref<Slot | undefined>
isHidden: boolean
key: symbol
}

/** A hovered element with its corresponding tooltip. */
export interface HoveredElement {
element: HTMLElement
entry: TooltipEntry
}

export type TooltipRegistry = ReturnType<typeof useTooltipRegistry>
export const [provideTooltipRegistry, useTooltipRegistry] = createContextStore(
'tooltip registry',
() => {
type EntriesSet = ShallowReactive<Set<TooltipEntry>>
// A map of hovered elements to their corresponding tooltips.
// There can be multiple tooltips for the same element, because we can have
// multiple nested tooltip triggers (components calling `registerTooltip`).
// The last hovered element is always on top of the map.
const hoveredElements = shallowReactive<Map<HTMLElement, EntriesSet>>(new Map())

const lastHoveredElement = computed(() => {
return iter.last(hoveredElements.keys())
/** The last hovered element with its corresponding tooltip. Undefined if no element with tooltip is hovered. */
const lastHoveredElement = computed<HoveredElement | undefined>(() => {
const lastKey = iter.last(hoveredElements.keys())
if (lastKey == null) return undefined
const entries = hoveredElements.get(lastKey)
assert(entries != null, 'entries is never null if lastKey is not null')
const lastEntry = iter.last(entries)
if (lastEntry == null) return undefined
return { element: lastKey, entry: lastEntry }
})

return {
lastHoveredElement,
getElementEntry(el: HTMLElement | undefined): TooltipEntry | undefined {
const set = el && hoveredElements.get(el)
return set ? iter.last(set) : undefined
},
/** Registers a tooltip and returns methods to control it. See `TooltipTrigger` component for usage. */
registerTooltip(slot: Ref<Slot | undefined>) {
const entry: TooltipEntry = {
contents: slot,
key: Symbol(),
}
const key = Symbol()
const registeredElements = new Set<HTMLElement>()
onUnmounted(() => {
for (const el of registeredElements) {
Expand All @@ -44,22 +57,43 @@ export const [provideTooltipRegistry, useTooltipRegistry] = createContextStore(
})

const methods = {
/** The registered tooltip must be shown when hovering this element. */
onTargetEnter(target: HTMLElement) {
const entriesSet: EntriesSet = hoveredElements.get(target) ?? shallowReactive(new Set())
entriesSet.add(entry)
entriesSet.add({ contents: slot, isHidden: false, key })
// make sure that the newly entered target is on top of the map
hoveredElements.delete(target)
hoveredElements.set(target, entriesSet)
registeredElements.add(target)
},
/** The registered tooltip must be hidden when finishing hovering this element. */
onTargetLeave(target: HTMLElement) {
const entriesSet = hoveredElements.get(target)
entriesSet?.delete(entry)
if (entriesSet) {
for (const e of entriesSet) {
if (e.key === key) entriesSet.delete(e)
}
}
Comment on lines +72 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have quite a lot of entries in this set - could it be a map?

Also, why didn't the previous approach work? In general I feel a bit more documentation would be helpful; for example I don't get why hoveredElements is map of HTMLElement -> Set - do we have many tooltips registered for single HTML element?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The set is usually just one element long. But in case of nested tooltip triggers (which are extremely rare) it can be longer.

registeredElements.delete(target)
if (entriesSet?.size === 0) {
hoveredElements.delete(target)
}
},
/**
* Forcefully hides the registered tooltip.
* Useful when we need to hide the tooltip without moving the mouse out of the element,
* like when clicking on a button.
*
* If several tooltips are registered for the same element, all of them will hide.
*/
forceHide() {
for (const el of registeredElements) {
const entriesSet = hoveredElements.get(el)
const newSet = new Set(entriesSet)
newSet.forEach((entry) => (entry.isHidden = true))
hoveredElements.set(el, newSet)
}
},
}
return methods
},
Expand Down
2 changes: 1 addition & 1 deletion app/gui/src/project-view/util/equals.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Same as `===` operator - used as functino parameter, */
/** Same as `===` operator - used as function parameter. */
export function defaultEquality(a: unknown, b: unknown): boolean {
return a === b
}
Expand Down
Loading