[TAS-5490] 🚸 Hide context menu when annotation menu opens#879
[TAS-5490] 🚸 Hide context menu when annotation menu opens#879nwingt wants to merge 3 commits intolikecoin:developfrom
Conversation
commit e33fa04 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 9555c22 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit baf9093 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
There was a problem hiding this comment.
Pull request overview
This PR updates the EPUB reader’s text-selection/annotation UX to avoid conflicts between the native context menu and the in-app annotation menu, building on the Android annotation-menu fixes from #875.
Changes:
- Add iOS/Android detection via
useAppDetection()and use it to gate platform-specific behavior. - In EPUB reader, disable native touch callout/context menu in the iframe and adjust navigation guards when the annotation menu is open.
- Improve selection handling by reacting to
selectionchange(debounced) and introducing a temporary highlight on iOS when clearing selection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pages/reader/epub.vue | Adds mobile/iOS-specific selection handling, prevents context menu conflicts, and introduces temporary highlights while the annotation menu is open. |
| composables/use-app-detection.ts | Extends app detection to expose isIOS and isAndroid flags. |
| components/AnnotationMenu.vue | Adjusts menu placement logic with an iOS-specific rule. |
| const debouncedSelectionChange = useDebounceFn(() => { | ||
| handleTextSelection(view.window) | ||
| }, 300) | ||
| removeSelectionChangeListener = useEventListener(view.window.document, 'selectionchange', debouncedSelectionChange) | ||
|
|
There was a problem hiding this comment.
The debounced selectionchange handler will still run after handleTextSelection() clears the selection on iOS (selection.removeAllRanges()), and when it fires it will see a collapsed selection and set isAnnotationMenuVisible to false. This can cause the annotation menu to immediately close on iOS after opening. Consider suppressing/canceling the debounced selectionchange callback when you intentionally clear the selection (e.g., keep a suppression flag/timestamp through the debounce window or call debouncedSelectionChange.cancel() before removeAllRanges()).
| removeTempHighlight() | ||
| if (!rendition.value) return | ||
| try { | ||
| rendition.value.annotations.highlight(cfi, {}, undefined, 'highlight', TEMP_HIGHLIGHT_STYLE) | ||
| tempHighlightCfi = cfi |
There was a problem hiding this comment.
addTempHighlight() uses epub.js annotation type 'highlight', and removeTempHighlight() removes by (cfi, 'highlight'). If the selected CFI matches an existing saved highlight, removing the temp highlight can remove the persisted highlight too. Use a distinct type/class for temporary highlights (or another unique identifier/removal mechanism) to avoid collisions.
e33fa04 to
995b3a5
Compare
commit 995b3a5 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 9555c22 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit baf9093 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
composables/use-app-detection.ts
Outdated
|
|
||
| onMounted(() => { | ||
| isAppUserAgent.value = navigator.userAgent?.startsWith('3ook-com-app') || false | ||
| isIOS.value = /ios|iPad|iPhone|iPod/i.test(navigator.userAgent) || (navigator.platform === 'MacIntel' && navigator.maxTouchPoints > 1) |
There was a problem hiding this comment.
The iOS user-agent regex includes /ios/i, which can false-positive on user agents like "KaiOS" (contains "iOS"). Consider narrowing this to iPhone/iPad/iPod (plus the iPadOS MacIntel touch-points heuristic) to avoid mis-detecting non-iOS devices as iOS.
| isIOS.value = /ios|iPad|iPhone|iPod/i.test(navigator.userAgent) || (navigator.platform === 'MacIntel' && navigator.maxTouchPoints > 1) | |
| isIOS.value = /\b(iPhone|iPad|iPod)\b/i.test(navigator.userAgent) || (navigator.platform === 'MacIntel' && navigator.maxTouchPoints > 1) |
995b3a5 to
3a82810
Compare
commit 3a82810 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 499ce9c Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit d050386 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
pages/reader/epub.vue
Outdated
| const TEMP_HIGHLIGHT_TYPE = 'underline' | ||
| const TEMP_HIGHLIGHT_STYLE = { 'fill': 'rgba(0, 227, 194, 0.3)', 'fill-opacity': '1' } | ||
| let tempHighlightCfi: string | null = null | ||
|
|
||
| function removeTempHighlight() { | ||
| if (!tempHighlightCfi || !rendition.value) return | ||
| try { | ||
| rendition.value.annotations.remove(tempHighlightCfi, TEMP_HIGHLIGHT_TYPE) | ||
| } | ||
| catch { | ||
| // Ignore | ||
| } | ||
| tempHighlightCfi = null | ||
| } | ||
|
|
||
| function addTempHighlight(cfi: string) { | ||
| removeTempHighlight() | ||
| if (!rendition.value) return | ||
| try { | ||
| rendition.value.annotations.underline(cfi, {}, undefined, TEMP_HIGHLIGHT_TYPE, TEMP_HIGHLIGHT_STYLE) | ||
| tempHighlightCfi = cfi |
There was a problem hiding this comment.
The temporary highlight uses annotations.underline(...) but the provided style uses fill/fill-opacity, which generally won’t affect underline annotations (they’re rendered with stroke). Also TEMP_HIGHLIGHT_TYPE is being used both as the underline type for remove(...) and as the className argument for underline(...), which is easy to mix up later. Consider switching the style to stroke-based properties and splitting into separate constants (e.g., annotation type vs CSS class name) to avoid accidental misuse.
composables/use-app-detection.ts
Outdated
| const ua = navigator.userAgent || '' | ||
| isAppUserAgent.value = ua.startsWith('3ook-com-app') | ||
| // Detect from RN WebView UA: "3ook-com-app/<version> (ios|android)" | ||
| const appPlatform = ua.match(/^3ook-com-app\/[\d.]+ \((\w+)\)/)?.[1] |
There was a problem hiding this comment.
appPlatform is compared case-sensitively (=== 'ios'/'android'). If the app UA ever uses iOS/Android (or other casing), platform detection will fail and the downstream reader behavior won’t apply (and the fallback /iPad|iPhone|iPod/ check may not match a fully custom UA). Consider normalizing the captured platform with .toLowerCase() before comparison.
| const appPlatform = ua.match(/^3ook-com-app\/[\d.]+ \((\w+)\)/)?.[1] | |
| const appPlatform = ua.match(/^3ook-com-app\/[\d.]+ \((\w+)\)/)?.[1]?.toLowerCase() |
components/AnnotationMenu.vue
Outdated
|
|
||
| const shouldAppearFromBottom = computed(() => props.position.y > viewportHeight.value / 2) | ||
| const isInBottomHalfViewport = computed(() => props.position.y > viewportHeight.value / 2) | ||
| const shouldAppearFromBottom = computed(() => !isIOS.value ? !isInBottomHalfViewport.value : isInBottomHalfViewport.value) |
There was a problem hiding this comment.
shouldAppearFromBottom uses a nested negation/ternary (!isIOS ? !isInBottomHalfViewport : isInBottomHalfViewport) which is hard to reason about and easy to invert accidentally later. Consider rewriting it to an equivalent but clearer expression (e.g., isIOS ? isInBottomHalfViewport : !isInBottomHalfViewport).
| const shouldAppearFromBottom = computed(() => !isIOS.value ? !isInBottomHalfViewport.value : isInBottomHalfViewport.value) | |
| const shouldAppearFromBottom = computed(() => isIOS.value ? isInBottomHalfViewport.value : !isInBottomHalfViewport.value) |
| // NOTE: Ignore page-turn if annotation was clicked or menu is open | ||
| if (isAnnotationClickInProgress.value || isAnnotationMenuVisible.value) { | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
When isAnnotationMenuVisible is true, the click handler returns early to avoid page turns, but it also means tapping on the page won’t dismiss the annotation menu. This is especially problematic on iOS where the selection is cleared (removeAllRanges()), so there’s no subsequent selectionchange to auto-hide the menu. Consider treating a page tap as “dismiss menu” (set isAnnotationMenuVisible false) while still preventing navigation.
| // NOTE: Ignore page-turn if annotation was clicked or menu is open | |
| if (isAnnotationClickInProgress.value || isAnnotationMenuVisible.value) { | |
| return | |
| } | |
| // NOTE: Ignore page-turn if annotation was clicked or menu is open. | |
| // If the menu is open, treat a page tap as "dismiss menu" while still | |
| // preventing navigation on this click. | |
| if (isAnnotationClickInProgress.value) { | |
| return | |
| } | |
| if (isAnnotationMenuVisible.value) { | |
| isAnnotationMenuVisible.value = false | |
| return | |
| } |
| textCSS.color = bodyCSS.color as string | ||
| textCSS['background-color'] = 'transparent !important' | ||
| } | ||
| if (isIOS.value || isAndroid.value) { |
There was a problem hiding this comment.
-webkit-touch-callout: none is being applied for all iOS/Android user agents (including mobile browsers), which disables the native selection/copy callout. If the intent is to avoid conflicts only inside the RN app, consider gating this by isApp (e.g., isApp && (isIOS || isAndroid)) so the web reader keeps expected copy/share behavior.
| if (isIOS.value || isAndroid.value) { | |
| const isRNWebView = | |
| typeof window !== 'undefined' && 'ReactNativeWebView' in window | |
| if ((isIOS.value || isAndroid.value) && isRNWebView) { |
| } | ||
| if (isIOS.value || isAndroid.value) { | ||
| removeContextMenuListener = useEventListener(view.window, 'contextmenu', (event: Event) => { | ||
| event.preventDefault() |
There was a problem hiding this comment.
The contextmenu handler currently calls preventDefault() unconditionally on iOS/Android, which can block long-press actions (copy, lookup, open-in-new-tab) even when the annotation menu isn’t involved. Consider preventing the context menu only while text selection/annotation menu is active, or scope this to app-only (isApp) to avoid a web UX regression.
| event.preventDefault() | |
| const selection = view.window.getSelection?.() | |
| const selectedText = selection?.toString() || '' | |
| if (selectedText.length > 0) { | |
| event.preventDefault() | |
| } |
| // On iOS, clear selection to dismiss native callout menu and add temp highlight | ||
| if (isIOS.value) { | ||
| isClearingSelection = true | ||
| selection.removeAllRanges() | ||
| addTempHighlight(cfiRange) | ||
| setTimeout(() => { |
There was a problem hiding this comment.
On iOS, selection.removeAllRanges() clears the selection immediately after it’s made. Since the AnnotationMenu currently doesn’t offer a Copy action, this effectively removes the user’s ability to copy selected text on iOS browsers. If suppressing the native callout is required, consider adding an explicit Copy action to the annotation menu (and/or only clearing selection in app mode) so copy remains possible.
commit 3a82810 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 499ce9c Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit d050386 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
commit 3a82810 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 499ce9c Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit d050386 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
commit 3a82810 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 499ce9c Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit d050386 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
|
@williamchong i ll rebase from #885 |
commit 3a82810 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit 499ce9c Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit d050386 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android
3a82810 to
ea83075
Compare
commit ea83075 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Prevent context & annotation menu conflict in iOS commit d5eb8da Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Prevent context & annotation menu conflict commit e2df7c5 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Thu Mar 26 15:26:58 2026 +0800 🐛 Fix annotation menu in Android # Conflicts: # composables/use-app-detection.ts
ea83075 to
4e5bf51
Compare
commit 4e5bf51 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:45:12 2026 +0800 🚸 Hide context menu when annotation menu opens for iOS commit 76d8dfc Author: Ng Wing Tat, David <i@ngwingt.at> Date: Fri Mar 27 10:44:39 2026 +0800 🚸 Hide context menu when annotation menu open commit 340b9f2 Author: Ng Wing Tat, David <i@ngwingt.at> Date: Mon Mar 30 16:51:34 2026 +0800 🚸 Prevent context & annotation menu conflict
| if (isIOS.value || isAndroid.value) { | ||
| textCSS['-webkit-touch-callout'] = 'none' | ||
| } |
There was a problem hiding this comment.
applyTheme() sets -webkit-touch-callout: none for all iOS/Android user agents. Combined with the global contextmenu suppression, this effectively disables the native long-press callout across the whole reader (including mobile web), not only while the annotation menu is shown. If the goal is just to avoid overlapping menus, consider scoping this behavior to app/webview contexts and/or toggling it only when the annotation menu is visible.
| if (isIOS.value || isAndroid.value) { | |
| textCSS['-webkit-touch-callout'] = 'none' | |
| } |
| }) | ||
|
|
||
| return { isApp } | ||
| const isIOS = computed(() => appOSName === 'iOS' || /iPhone|iPad/.test(userAgent)) |
There was a problem hiding this comment.
isIOS only checks appOSName === 'iOS' or /iPhone|iPad/, which will miss some iOS/iPadOS user agents (e.g. iPod, and iPadOS in “desktop mode” where UA can look like macOS). If this composable is used to drive iOS-specific behavior, consider expanding the UA checks (e.g. include iPod, and a client-only iPadOS heuristic such as platform === 'MacIntel' && maxTouchPoints > 1).
| const isIOS = computed(() => appOSName === 'iOS' || /iPhone|iPad/.test(userAgent)) | |
| const isIOS = computed(() => { | |
| if (appOSName === 'iOS') { | |
| return true | |
| } | |
| // Match common iOS devices by user agent | |
| if (/iPhone|iPad|iPod/.test(userAgent)) { | |
| return true | |
| } | |
| // Detect iPadOS in "desktop mode" where UA may look like macOS | |
| if (import.meta.client) { | |
| const nav = window.navigator | |
| if (nav && nav.platform === 'MacIntel' && (nav as any).maxTouchPoints > 1) { | |
| return true | |
| } | |
| } | |
| return false | |
| }) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
components/AnnotationMenu.vue:63
shouldAppearFromBottomis now alwaystrueon non‑iOS (!isIOS.value || ...), which forces the annotation menu to always render below the selection. This can cause the menu to go off-screen when the selection is near the bottom of the viewport (and it also changes behavior for Android/desktop compared to the previous positioning logic). Consider basing the value on available space (e.g., keep using the viewport half heuristic for non‑iOS, or compute whether there’s enough room below vs above) so the menu flips above when near the bottom edge.
const isInBottomHalfViewport = computed(() => props.position.y > viewportHeight.value / 2)
const shouldAppearFromBottom = computed(() => !isIOS.value || isInBottomHalfViewport.value)
const menuStyle = computed(() => {
const minX = menuWidth.value / 2 + MENU_PADDING * 2
const maxX = viewportWidth.value - menuWidth.value / 2 - MENU_PADDING * 2
const clampedX = menuWidth.value > 0 && viewportWidth.value > 0
? minX > maxX ? viewportWidth.value / 2 : Math.min(Math.max(props.position.x, minX), maxX)
: props.position.x
return {
padding: `${MENU_PADDING}px`,
left: `${clampedX}px`,
top: `${shouldAppearFromBottom.value ? props.position.yBottom + MENU_PADDING : props.position.y - MENU_PADDING}px`,
transform: shouldAppearFromBottom.value ? 'translateX(-50%)' : 'translate(-50%, -100%)',
| @@ -1,15 +1,26 @@ | |||
| const APP_USER_AGENT_PREFIX = '3ook-com-app' | |||
| // e.g. "3ook-com-app/1.1.0 (iOS 18.0) Build/42" | |||
| const APP_USER_AGENT_REGEX = /^3ook-com-app\/[\d.]+ \((iOS|Android) [\d.]+\)/ | |||
There was a problem hiding this comment.
Do you need this to match old version like 3ook-com-app/1.1.0 or 3ook-com-app/1.1.0 (ios 18.0) also?
Based on #875, #885, likecoin/3ook-com-app#26