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

foreground-service: Listen to tab visibility change event #1878

Merged
merged 1 commit into from
May 7, 2023

Conversation

florian-h05
Copy link
Contributor

Fixes #1872.

This makes the foreground service also listen to the visibilitychange event to stop and resume foreground activity when the tab is changed or the browser is closed.

See https://developer.mozilla.org/en-US/docs/Web/API/Document/visibilitychange_event.

Fixes openhab#1872.

This makes the foreground service also listen to the `visibilitychange` event to stop and resume foreground activity when the tab is changed or the browser is closed.

See https://developer.mozilla.org/en-US/docs/Web/API/Document/visibilitychange_event.

Signed-off-by: Florian Hotze <[email protected]>
@florian-h05 florian-h05 requested a review from a team as a code owner May 7, 2023 12:29
@florian-h05
Copy link
Contributor Author

FYI @digitaldan

@relativeci
Copy link

relativeci bot commented May 7, 2023

Job #969: Bundle Size — 15.76MiB (+0.02%).

dba6859(current) vs 58dbbc9 main#968(baseline)

⚠️ Bundle contains 16 duplicate packages

Metrics (3 changes)
                 Current
Job #969
     Baseline
Job #968
Initial JS 1.73MiB(+0.03%) 1.73MiB
Initial CSS 608.89KiB(~+0.01%) 608.87KiB
Cache Invalidation 93.94% 93.95%
Chunks 219 219
Assets 689 689
Modules 1718 1718
Duplicate Modules 85 85
Duplicate Code 1.73% 1.73%
Packages 137 137
Duplicate Packages 15 15
Total size by type (3 changes)
                 Current
Job #969
     Baseline
Job #968
CSS 858.62KiB (~+0.01%) 858.61KiB
Fonts 526.1KiB 526.1KiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.25MiB (+0.02%) 9.25MiB
Media 295.6KiB 295.6KiB
Other 4.73MiB (+0.02%) 4.73MiB

View job #969 reportView main branch activity

Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

Cool! I'm all for avoiding things happening in the background when they're not useful, especially when the UI is used on a battery-powered device, like a phone or tablet.

@ghys ghys merged commit a548e0d into openhab:main May 7, 2023
@ghys ghys added enhancement New feature or request main ui Main UI labels May 7, 2023
@ghys ghys added this to the 4.0 milestone May 7, 2023
@florian-h05 florian-h05 deleted the bug-foreground-mixin branch May 7, 2023 13:14
@florian-h05
Copy link
Contributor Author

Totally agreed 👍

I noticed this issue especially when using the SIP client in the iOS app: When I switched to another app, mic access stayed active. This is quite confusing and I don’t feel so good when the yellow indicator is displaying that something is listening to me.

@ghys
Copy link
Member

ghys commented May 7, 2023

It was even a bigger problem for UIs that just listen to /rest/events SSEs unfiltered (like HABPabel, HABot or OH2's Paper UI) - in any decent-sized installation there's a lot of traffic there and most of it isn't useful for displaying the current page in the UI, so that's why I introduced the POST /rest/events/states/* endpoints for end-user pages and most settings pages maintain their own SSE connections to filter out unwanted events.

It could even be useful to close the SSE connection and resume it (with the current values) on visibilitychange.

@dilyanpalauzov
Copy link

With this change, will the SIP widget remain active, when the user changes the tab, and later returns? It shall stay active.

@florian-h05
Copy link
Contributor Author

It could even be useful to close the SSE connection and resume it (with the current values) on visibilitychange.

I was not thinking about SSE when I created this PR, but very good points! I'm not sure what SSE currently does when the tab is switched, however thanks to my PRs with the toast message and the SSE reconnection mechanism I have noticed that once I leave Safari on iOS and come back, SSE has to reconnect so it seems that it is not always connected.

With this change, will the SIP widget remain active, when the user changes the tab, and later returns? It shall stay active.

With this change, it will unregister when the user changes the tab, and then reconnect once he returns. As long as the user is in another tab or has closed the browser window, the SIP widget is disconnected.

@dilyanpalauzov
Copy link

As long as the user is in another tab ... the SIP widget is disconnected.

This is not good. It shall be possible to receive calls in inactive tabs and it shall be possible during a call to change the tabs.

@florian-h05
Copy link
Contributor Author

florian-h05 commented May 7, 2023

There are also good arguments to have that behaviour. The SIP widget is the only part of MainUI that might have a benefit from staying active in the background.

If you want the ability to not have it always active, feel free to implement something.
You could make it configurable what happens when the page is left or something else, just keep battery usage, bandwidth, browser limitations and the average user in mind.

Just note that MainUI is meant to be a PWA to control openHAB and your home, it is not meant to be a phone app.

ghys pushed a commit that referenced this pull request May 9, 2023
…#1885)

... and check for foreground before starting foreground activity.

Follow-up for #1878.

Signed-off-by: Florian Hotze <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SIP does not unregister when page is left
3 participants