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 race condition when dismissing the gallery causing the UI to be blocked #3037

Merged
merged 4 commits into from Feb 20, 2024

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented Feb 19, 2024

🔗 Issue Links

Will fix #2975

🎯 Goal

Fix UI getting blocked after swiping up the gallery view.

🛠 Implementation

The issue is that a cancellation and a regular dismissal were happening at the same time, causing a race condition. A cancel was getting called twice, and with completeTransition(true), causing the transition to finish when it should not.

🧪 Manual Testing Notes

  1. Open a channel
  2. Open a image attachment
  3. Swipe up multiple times very fast until the gallery is dismissed
  4. The UI should not be blocked

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero ⚠️ policy (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@nuno-vieira nuno-vieira added 🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK labels Feb 19, 2024
@nuno-vieira nuno-vieira requested a review from a team as a code owner February 19, 2024 18:38
@nuno-vieira nuno-vieira changed the title Fix UI blocked when swiping up the gallery view Fix race condition when dismissing the gallery causing the UI to be blocked Feb 19, 2024
Copy link

sonarcloud bot commented Feb 19, 2024

Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

LGTM ✅ Just please update the changelog wording before merging.

CHANGELOG.md Outdated
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Fix `CGBitmapContextInfoCreate` console log warning when creating merged channel avatars [#3018](https://github.com/GetStream/stream-chat-swift/pull/3018)
- Slight performance improvement in the message list by caching `NSRegularExpression` in `MarkdownFormatter` [#3020](https://github.com/GetStream/stream-chat-swift/pull/3020)
- Slight performance improvement in the message list by skipping channel list updates when it is not visible [#3021](https://github.com/GetStream/stream-chat-swift/pull/3021)
- Fix race condition when dismissing the gallery causing the UI to be blocked [#3037](https://github.com/GetStream/stream-chat-swift/pull/3037)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to clarify it's an edge case. Reading it like this sounds like it was common.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it is a race condition, usually it is already rare 🤔

@nuno-vieira nuno-vieira merged commit e94f903 into develop Feb 20, 2024
5 checks passed
@nuno-vieira nuno-vieira deleted the fix/ui-blocked-after-swipe-up-gallery branch February 20, 2024 10:31
@nuno-vieira nuno-vieira mentioned this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Whole interface blocked when swipe photo in fullscreen mode
3 participants