Skip to content

🚸 Prevent context & annotation menu conflict#894

Open
nwingt wants to merge 1 commit intolikecoin:developfrom
nwingt:feature/annotation
Open

🚸 Prevent context & annotation menu conflict#894
nwingt wants to merge 1 commit intolikecoin:developfrom
nwingt:feature/annotation

Conversation

@nwingt
Copy link
Copy Markdown
Member

@nwingt nwingt commented Mar 30, 2026

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to avoid conflicts between the system context menu and the in-app annotation menu by adjusting platform detection (iOS/Android) and using that detection to alter annotation menu placement behavior.

Changes:

  • Update useAppDetection() to parse a richer app User-Agent format and expose isIOS/isAndroid.
  • Adjust AnnotationMenu positioning logic based on iOS detection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
composables/use-app-detection.ts Switches app detection to a regex-based UA match and adds isIOS/isAndroid computed refs.
components/AnnotationMenu.vue Uses iOS detection to influence whether the annotation menu appears above vs below the selection.

Comment on lines +1 to 16
// 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.]+\)/

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'
})
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

Choose a reason for hiding this comment

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

It is safe


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

Choose a reason for hiding this comment

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

it is intended

@nwingt nwingt marked this pull request as ready for review March 30, 2026 13:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@@ -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

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?

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