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

Don't make notification window key as it could conflict with the main window #50

Merged
merged 2 commits into from
Dec 18, 2020

Conversation

kairadiagne
Copy link
Contributor

@kairadiagne kairadiagne commented Dec 18, 2020

What's the problem?

We ran into an issue with an Authentication library and ASWebAuthenticationSession. The library would automatically use the applications keyWindow as the presentation anchor for the authentication session. If someone would trigger a login while a notification was presented the safari view controller presented by the authentication session would disappear as soon as the notification was dismissed.

What was causing the issue?

This happened because the notification window was the apps keyWindow while a notification was presented. As only one window at a time can be the key window the auth SDK was picking that window as the presentation anchor of the auth session. When the notification was dismissed the apps main window became key again and that would break the authentication flow.

How it was solved?

It turns out that the UINotificationWindow does not have to be the key window. The only thing it needs is touch events, and those are always delivered to the window in which it occurred. The only requirement for that is that the window is visible. We also want notifications to be presented on top of the app. This is already managed by setting an appropriate window level for presentation (by default above the status bar).

The only events that the window can't handle if it is not key are:

  • Motion (e.g. shake gesture)
  • keyboard (e.g. text field)
  • remote (not applicable)
  • presses (physical key presses).

Those are not relevant for the use case of presenting a simple in app notification.

@kairadiagne kairadiagne self-assigned this Dec 18, 2020
@kairadiagne kairadiagne marked this pull request as draft December 18, 2020 11:51
@wetransferplatform
Copy link
Collaborator

wetransferplatform commented Dec 18, 2020

Messages
📖

View more details on Bitrise

📖 UINotifications: Executed 24 tests, with 0 failures (0 unexpected) in 2.204 (2.262) seconds

UINotifications.framework: Coverage: 97.93

File Coverage
UINotificationPresentationContext.swift 100.0%
UINotificationCenter.swift 100.0%

Generated by 🚫 Danger Swift against be26c30

For the presentation of notifications the window does not have to be key.
It will still receive touch events (gestures, buttons etc) but won't receive keyboard events and motion events.
@kairadiagne kairadiagne marked this pull request as ready for review December 18, 2020 12:34
@kairadiagne kairadiagne merged commit 12d943c into master Dec 18, 2020
@kairadiagne kairadiagne deleted the feature/window-management branch December 18, 2020 13:42
@wetransferplatform
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 1.4.1 🚀

Generated by GitBuddy

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