Skip to content

🚸 Prevent context & annotation menu conflict #894

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.

Sign up for GitHub

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

Open
nwingt wants to merge 1 commit into
base: develop
Choose a base branch
from
+21 −7
Open

🚸 Prevent context & annotation menu conflict #894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion components/AnnotationMenu.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
<script setup lang="ts">
import { ANNOTATION_COLORS, ANNOTATION_INDICATOR_COLORS_MAP } from '~/constants/annotations'

const { isIOS } = useAppDetection()

const MENU_PADDING = 8

const props = defineProps<{
Expand All @@ -45,7 +47,8 @@ const menuEl = useTemplateRef<HTMLDivElement>('menuEl')
const { width: menuWidth } = useElementSize(menuEl)
const { width: viewportWidth, height: viewportHeight } = useWindowSize()

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)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

shouldAppearFromBottom is now !isIOS || isInBottomHalfViewport, which makes it always true on non‑iOS devices (menu always positioned below the selection) and no longer depends on viewport position like before. If the intent was to change behavior only on iOS, this boolean is inverted; adjust the condition so non‑iOS retains the original isInBottomHalfViewport logic.

Suggested change
const shouldAppearFromBottom = computed(() => !isIOS.value || isInBottomHalfViewport.value)
const shouldAppearFromBottom = computed(() =>
isIOS.value ? true : isInBottomHalfViewport.value,
)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@nwingt nwingt Mar 30, 2026

Choose a reason for hiding this comment

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

it is intended


const menuStyle = computed(() => {
const minX = menuWidth.value / 2 + MENU_PADDING * 2
Expand Down
23 changes: 17 additions & 6 deletions composables/use-app-detection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.]+\)/
Copy link
Copy Markdown
Member

@williamchong williamchong Mar 30, 2026

Choose a reason for hiding this comment

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

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?


export function useAppDetection() {
const getRouteQuery = useRouteQuery()

const isAppUserAgent = import.meta.server
? useRequestHeaders(['user-agent'])['user-agent']?.startsWith(APP_USER_AGENT_PREFIX) || false
: navigator.userAgent?.startsWith(APP_USER_AGENT_PREFIX) || false
const userAgent = import.meta.server
? useRequestHeaders(['user-agent'])['user-agent'] || ''
: navigator.userAgent || ''

const appUAMatches = userAgent.match(APP_USER_AGENT_REGEX)
const appOSName = appUAMatches?.[1]

const isApp = computed(() => {
return isAppUserAgent || getRouteQuery('app') === '1'
return !!appUAMatches || getRouteQuery('app') === '1'
})
Comment on lines +1 to 16
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

isApp detection is now based on APP_USER_AGENT_REGEX matching, which is strictly narrower than the previous startsWith('3ook-com-app') check. Any UA that still uses the old prefix-only format (or otherwise deviates from the exact regex) will stop being detected as app unless ?app=1 is present. Consider keeping a prefix startsWith fallback (or loosening the regex) so existing app versions don’t regress.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@nwingt nwingt Mar 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It is safe


return { isApp }
const isIOS = computed(() => appOSName === 'iOS' || /iPhone|iPad/.test(userAgent))
const isAndroid = computed(() => appOSName === 'Android' || /Android/.test(userAgent))

return {
isApp,
isIOS,
isAndroid,
}
}
Loading