-
Notifications
You must be signed in to change notification settings - Fork 486
Exposing an event that can keep track of losing and gaining focus in app #681
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
Conversation
|
|
We already expose "use present" and "app activity state". Is it really necessary to add a "main window is in focus" concept as well? I'm not convinced. I'd prefer to combine this with one of the existing concepts if possible. I also agree with Radu's other points. This concept, as it's currently defined, doesn't take into account multi-window scenarios. |
|
@reseul @erictraut thanks for the clarification that events are not emitted in existing RNW. Looks like we cant use this approach for UWP Regarding App activity state - we only know when app is minimized as Backgrounded. There is no concept of not in focus. Regarding User Presence - this is tighlty related to user activity. Currently is user is focused on the App and not doing an "mouse moves" or clicks - user presence becomes false. There are a bunch of scenarios where exposing that the app is focused or not becomes helpful. And we blocked on this availability. We have a gap in not being able to detect focused and not in focus. Regarding multi windows - am happy to discuss ideas. We can think of an api to detect
|
|
Can you provide some concrete examples of scenarios that are blocked on not knowing whether the frontmost window is in focus or not? I think we need to carefully document exactly what is meant by app activity and user presence. It has been rather loose in the past. The definition is complicated by the differences between mobile and desktop (windowed) UI's. Once that's documented, we can determine whether we can amend or extend one of these concepts or whether a third concept ("front window in focus") is needed. I'd prefer to avoid adding a third concept unless there's a really compelling reason to do so. With regard to multi-window... ReactXP doesn't currently have the notion of multiple windows. It simply has the notion of "root views" and rootViewId's. Multiple root views can appear together within a single window or be in different windows. I don't want to expose the concept of windows and windowId's if we can help it. Related to this entire discussion... After investigating it further, I think we should eliminate the dependency on ifvisible. It's not buying us much, and it's adding a bunch of extra code to reactxp's package size. |
|
Sure. There are a few scenarios that comes to my mind in Skype where this is currently needed:
For now we are using UserPresence for last two scenarios. This means that when user is focused in the app and is not interactive with the app - it would treat is as the same as when it is not focused. This is a bit confusing. I don’t think the people who implemented this knew of this issue in my feeling. And currently there is also another bug, that when user is not focused on the app, but hovers on top if it - it is considered that the user is active for browser/electron.(There is a PR that is addressing it) Regarding documenting what the behaviour is - here is a brief summary of its behaviour:
For mobile:
UserPresence: For electron/web .
For all native platforms including RNUWp
Always true. So UserPresence is false doesn’t mean that the App is not in Focus. I was thinking we can user ‘Inactive’ to indicate out of focus for Electron/web. Seems like that is what RN is doing - the intermediate state between app is active and backgrounded is Inactive. @erictraut What do you think? |
|
And yes i will replace ifvisible package with this PR #680 - implementing changes now. |
|
Scenarios #2 and #3 above are the exact scenarios for which UserPresence was designed. If the user is not present at the computer or is not able to see the the app (because it's minimized or otherwise obscured), the UserPresence module should indicate "false". You're arguing if the main window is not in focus, we should assume the user is not focused on the app. That's point #3 in your definition of UserPresence. When I originally implemented this, I assumed that the user could have simply brought another app to the foreground while still monitoring the app in the background (e.g. on a second monitor). I often use the Skype app in this manner. However, I can also understand your point. We could modify the UserPresence definition (point #3) to return False when the app's main window isn't in focus. You're also arguing that we should change the behavior on point #4. That was also intentional because a mouse movement within the window (even if it's not focused) is a clear indication that app's window is visible (i.e. not entirely obscured by another window) and the user is present to see changes within the app. Here again, I could be convinced that we should change the behavior, but you should understand that the current behavior was intentional, not a bug. It works the way that I would expect the app to work. If other users have complained about the behavior, then I'm fine changing it. I don't feel that strongly about it. Now, lets go back to scenario #1. I see your point here. And I like your idea of using the "Inactive" state of app activity for desktop apps to indicate that the main window is no longer in focus. In summary, I think we're talking about three changes:
|
|
Thanks @erictraut. I am fine with leaving the user presence as false when the user does not interact with the app. I was'nt sure if it was by design, thanks for the clarification. Now since we expose Inactive state - leaving user presence as is makes sense. I have implemented the three specified changes in this PR #680 I will close this current PR. |
Currently we dont have a way to distinguish when the app has focus and when the app window has lost focused. This is useful in quite a number of scenarios.
Exposing an API that can explicitly track focus changes in app. On mobile platform it would be the same as RN.AppState.