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

fix(mobile): context menu crash when released too quickly #2732

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

hyoban
Copy link
Member

@hyoban hyoban commented Feb 12, 2025

Description

PR Type

  • Feature
  • Bugfix
  • Hotfix
  • Other (please describe):

Screenshots (if UI change)

Demo Video (if new feature)

Linked Issues

Additional context

Changelog

  • I have updated the changelog/next.md with my changes.

Copy link

vercel bot commented Feb 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
follow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 12, 2025 2:19pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
follow-external-ssr ⬜️ Ignored (Inspect) Feb 12, 2025 2:19pm

@follow-reviewer-bot
Copy link

Suggested PR Title:

fix: add workaround for context menu crash on long press

Change Summary:
Added a workaround to prevent context menu crash during long press actions. This modification improves the stability of the ItemPressable component when users interact with it rapidly.

Code Review:

  • File: apps/mobile/src/components/ui/pressable/item-pressable.tsx
    • Line 53: The comment mentions a workaround referencing a GitHub issue. However, there is no guarantee that this issue or workaround will remain relevant or that the referenced external dependency will behave consistently in the future. It would be better to include more context in the comment, such as the expected behavior change when the issue is resolved or explicitly track if/when this code should be revisited. Additionally, consider verifying if this workaround introduces any unintended side effects.
    • Line 54: The hardcoded default value for delayLongPress (100) might not align with user expectations across all use cases. If this behavior should be configurable at a higher level, document this choice and validate whether the value can be overridden globally in the app configuration. Alternatively, surface it explicitly to the caller for transparency.

Address these points to ensure maintainability and clarity of intent in the implementation.

@hyoban hyoban merged commit 1327f56 into dev Feb 12, 2025
10 checks passed
@hyoban hyoban deleted the fix/context-menu-crash branch February 12, 2025 14:34
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.

1 participant