From 72472f472094167b87d0e63ab8a66e7c8dc69704 Mon Sep 17 00:00:00 2001 From: Oliver Date: Thu, 25 Jul 2024 20:54:37 +0200 Subject: [PATCH] Improve on UX for highlighter (remove/hide/refresh) --- external/@worldbrain/memex-common | 2 +- src/content-scripts/content_script/global.ts | 8 ++ .../ribbon/react/containers/ribbon/logic.ts | 4 +- .../shared-state/shared-in-page-ui-state.ts | 15 ++-- .../content_script/components/index.tsx | 11 ++- .../tooltip/content_script/interactions.ts | 78 +++++++++++++++---- 6 files changed, 90 insertions(+), 28 deletions(-) diff --git a/external/@worldbrain/memex-common b/external/@worldbrain/memex-common index 834f636bba..f7227b35bb 160000 --- a/external/@worldbrain/memex-common +++ b/external/@worldbrain/memex-common @@ -1 +1 @@ -Subproject commit 834f636bba286366278c866d93f0b7d8a9d17ec6 +Subproject commit f7227b35bb94ddeb12aee78a69ee92a78dd4ae4b diff --git a/src/content-scripts/content_script/global.ts b/src/content-scripts/content_script/global.ts index e0a40c302b..03ef7b2bb0 100644 --- a/src/content-scripts/content_script/global.ts +++ b/src/content-scripts/content_script/global.ts @@ -1191,6 +1191,10 @@ export async function main( deleteAnnotation: async (annotationId) => { await annotationsFunctions.deleteAnnotation(annotationId) }, + tooltip: { + getState: tooltipUtils.getTooltipState, + setState: tooltipUtils.setTooltipState, + }, annotationsBG, annotationsCache, contentSharingBG, @@ -1448,6 +1452,10 @@ export async function main( }), askAI: annotationsFunctions.askAI(), createYoutubeTimestamp: annotationsFunctions.createYoutubeTimestamp, + tooltip: { + getState: tooltipUtils.getTooltipState, + setState: tooltipUtils.setTooltipState, + }, }) // 9. Check for page activity status diff --git a/src/in-page-ui/ribbon/react/containers/ribbon/logic.ts b/src/in-page-ui/ribbon/react/containers/ribbon/logic.ts index d2e79cb5cb..f5f0eea7f1 100644 --- a/src/in-page-ui/ribbon/react/containers/ribbon/logic.ts +++ b/src/in-page-ui/ribbon/react/containers/ribbon/logic.ts @@ -1063,15 +1063,13 @@ export class RibbonContainerLogic extends UILogic< setState(!currentSetting) try { + await this.dependencies.tooltip.setState(!currentSetting) if (currentSetting === true) { await this.dependencies.inPageUI.removeTooltip() } else { - await this.dependencies.tooltip.setState(!currentSetting) await this.dependencies.inPageUI.toggleTooltip() - await this.dependencies.inPageUI.showTooltip() } - await this.dependencies.tooltip.setState(!currentSetting) } catch (err) { setState(!currentSetting) throw err diff --git a/src/in-page-ui/shared-state/shared-in-page-ui-state.ts b/src/in-page-ui/shared-state/shared-in-page-ui-state.ts index e5d9d348e6..01352c0ec2 100644 --- a/src/in-page-ui/shared-state/shared-in-page-ui-state.ts +++ b/src/in-page-ui/shared-state/shared-in-page-ui-state.ts @@ -267,6 +267,10 @@ export class SharedInPageUIState implements SharedInPageUIInterface { } async showTooltip(options?: ToolTipActionOptions) { + if (this.componentsSetUp.tooltip) { + await this.options.loadComponent('tooltip') + } + const maybeEmitAction = () => { if (options) { this._emitAction({ @@ -275,12 +279,6 @@ export class SharedInPageUIState implements SharedInPageUIInterface { }) } } - - if (this.componentsShown.tooltip) { - maybeEmitAction() - return - } - await this._setState('tooltip', true) maybeEmitAction() return @@ -320,7 +318,10 @@ export class SharedInPageUIState implements SharedInPageUIInterface { } private async _setState(component: InPageUIComponent, visible: boolean) { - if (this.componentsShown[component] === visible) { + if ( + this.componentsShown[component] === visible && + component !== 'tooltip' + ) { return } diff --git a/src/in-page-ui/tooltip/content_script/components/index.tsx b/src/in-page-ui/tooltip/content_script/components/index.tsx index d6b5537955..746fdc19a8 100644 --- a/src/in-page-ui/tooltip/content_script/components/index.tsx +++ b/src/in-page-ui/tooltip/content_script/components/index.tsx @@ -48,7 +48,7 @@ import { cloneSelectionAsPseudoObject } from '@worldbrain/memex-common/lib/annot interface TooltipRootProps { mount: InPageUIRootMount - params: Omit + params: Props onTooltipInit: (showTooltip: () => void) => void toggleTooltipState: (state: boolean) => Promise analyticsBG: AnalyticsCoreInterface @@ -72,6 +72,11 @@ interface TooltipRootProps { preventHideTooltip?: boolean, ): Promise getWindow: () => Window + tooltip: { + getState: () => void + setState: (tooltipValue: boolean) => void + } + shouldInitTooltip: boolean } interface TooltipRootState { @@ -533,6 +538,8 @@ class TooltipRoot extends React.Component { } showColorPicker={this.state.showColorPicker} toggleTooltipState={props.toggleTooltipState} + tooltip={props.params.tooltip} + shouldInitTooltip={props.shouldInitTooltip} /> @@ -566,6 +573,8 @@ export function setupUIContainer( createHighlight={params.createHighlight} getWindow={params.getWindow} toggleTooltipState={props.toggleTooltipState} + tooltip={params.tooltip} + shouldInitTooltip={params.shouldInitTooltip} />, mount.rootElement, ) diff --git a/src/in-page-ui/tooltip/content_script/interactions.ts b/src/in-page-ui/tooltip/content_script/interactions.ts index 1e3268d10f..15e1b5cce8 100644 --- a/src/in-page-ui/tooltip/content_script/interactions.ts +++ b/src/in-page-ui/tooltip/content_script/interactions.ts @@ -57,6 +57,12 @@ let manualOverride = false let removeTooltipStateChangeListener: () => void +let shouldInitTooltip = true // You can set this based on your conditions + +export const setShouldInitTooltip = (value: boolean) => { + shouldInitTooltip = value +} + interface TooltipInsertDependencies extends TooltipDependencies { mount: InPageUIRootMount } @@ -103,6 +109,7 @@ export const insertTooltip = async (params: TooltipInsertDependencies) => { askAI: shortcutToKeyStrs(state.askAI), } }, + onTooltipHide: () => params.inPageUI.hideTooltip(), onTooltipClose: () => params.inPageUI.removeTooltip(), toggleTooltipState: async (state: boolean) => { @@ -154,7 +161,13 @@ export const insertTooltip = async (params: TooltipInsertDependencies) => { params.inPageUI.events.on( 'tooltipAction', (event, callback) => { - handleExternalAction(event, callback) + if (event.annotationCacheId || event.openForSpaces) { + handleExternalAction(event, callback) + callback(true) + } else { + setShouldInitTooltip(true) + } + callback(true) }, ) @@ -186,6 +199,11 @@ export const insertTooltip = async (params: TooltipInsertDependencies) => { // Placeholder function, replace with actual implementation }, openImageInPreview: async (src: string) => null, + tooltip: { + getState: params.tooltip.getState, + setState: params.tooltip.setState, + }, + shouldInitTooltip: shouldInitTooltip, }, { annotationsBG: params.annotationsBG, @@ -203,9 +221,13 @@ export const insertTooltip = async (params: TooltipInsertDependencies) => { createHighlight: params.createHighlight, getWindow: () => window, toggleTooltipState: params.toggleTooltipState, + tooltip: { + getState: params.tooltip.getState, + setState: params.tooltip.setState, + }, + shouldInitTooltip: shouldInitTooltip, }, ) - setupTooltipTrigger(() => { params.inPageUI.showTooltip() }, null) @@ -279,16 +301,35 @@ export const removeTooltip = (options?: { override?: boolean }) => { // } export const showContentTooltip = async (params: TooltipInsertDependencies) => { - if (!showTooltip) { + if (showTooltip == null) { await insertTooltip(params) } - if (userSelectedText()) { - const position = calculateTooltipPostion() - showTooltip(position) + // There seems to be a race condition with the tooltip setup function to get the "showtooltip" function + // This is a temporary fix to keep trying to get the showtooltip function until it is available + + let attempts = 0 + const maxAttempts = 20 // 5 seconds total + const baseDelay = 50 + + while (showTooltip == null && attempts < maxAttempts) { + await new Promise((resolve) => + setTimeout(resolve, baseDelay * Math.pow(1.5, attempts)), + ) + + attempts++ + } + + if (showTooltip == null) { + console.error( + 'Failed to initialize showTooltip after multiple attempts', + ) + return } -} + const position = calculateTooltipPosition() + showTooltip(position) +} /** * Checks for certain conditions before triggering the tooltip. * i) Whether the selection made by the user is just text. @@ -325,7 +366,7 @@ export const conditionallyTriggerTooltip = delayed( // } else if (positioning === 'mouse' && event) { // position = calculateTooltipPostion() // } - position = calculateTooltipPostion() + position = calculateTooltipPosition() analytics.trackEvent({ category: 'InPageTooltip', action: 'showTooltip', @@ -340,17 +381,22 @@ export const conditionallyTriggerTooltip = delayed( 10, ) -export function calculateTooltipPostion(): TooltipPosition { - const range = document.getSelection().getRangeAt(0) +export function calculateTooltipPosition(): TooltipPosition { + const selection = document.getSelection() + if (!selection || selection.rangeCount === 0) { + // Return a default position if there's no selection + return { x: window.innerWidth / 2, y: window.innerHeight / 2 } + } + + const range = selection.getRangeAt(0) const boundingRect = range.getBoundingClientRect() - // x = position of element from the left + half of it's width + + // x = position of element from the left + half of its width const x = boundingRect.left + boundingRect.width / 2 - // y = scroll height from top + pixels from top + height of element - offset + // y = scroll height from top + pixels from top + height of element const y = window.pageYOffset + boundingRect.top + boundingRect.height - return { - x, - y, - } + + return { x, y } } function isAnchorOrContentEditable(selected) {