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 share button not working for some users #5849

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

haileyok
Copy link
Member

Fixes #5719

Why

This is one that I was unable to reproduce for some time, but was finally able to "accidentally" discover. If you press too long on the "share" button, the dialog actually ends up staying open a tad bit longer. It seems like when that happens, the app isn't able to present the share dialog because the menu sheet is still presented and animating away.

Notably here, we were not calling our menu item onPress inside of the close() function's callback, but rather in parallel. We've seen in the past where its pretty essential to do that, and it seems like this is another one of those cases.

Test Plan

Before this PR, open the share sheet and take a bit longer on pressing the "Share" button. It shouldn't open the share sheet. Then, test this pr and do the same. It should still open.

Also worth checking other buttons like "Translate" to ensure they still work.

Comment on lines 123 to 128
if (e.defaultPrevented) {
await onPress(e)
} else {
control?.close(() => {
onPress(e)
})
Copy link
Member Author

Choose a reason for hiding this comment

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

@estrattonbailey would it be okay for us to remove this e.defaultPrevented() entirely for native? Are there cases where we want something to run before we close the sheet? I assume the defaultPrevented() is only useful on web but maybe I'm wrong.

Copy link

Old size New size Diff
7.9 MB 7.9 MB 0 B (0.00%)

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.

Sharing feed does not work on iOS
2 participants