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

Web UI shows "No devices yet" briefly during launch #3056

Open
justdave opened this issue Dec 18, 2022 · 4 comments · Fixed by #3123
Open

Web UI shows "No devices yet" briefly during launch #3056

justdave opened this issue Dec 18, 2022 · 4 comments · Fixed by #3123
Labels
Milestone

Comments

@justdave
Copy link

justdave commented Dec 18, 2022

This is primarily UX issue...

When you first open the web page, you get a message stating "No devices yet. Click + to scan for new devices." which remains on the screen for a few seconds before all of your devices load.

To the end user this gives a brief "oh no" moment when they might think it lost all of their devices.

This message should not appear unless the devices have finished loading and there actually aren't any.

Having a message is better than a blank screen though, so something that indicates it's still loading or looking for devices should be shown first, then switch it out with the above message once it's done loading if it didn't find any.

This is on 1.1 beta 2

@benfrancis benfrancis added the bug label Jan 2, 2023
@benfrancis benfrancis added this to the 1.2 milestone Jan 2, 2023
@benfrancis
Copy link
Member

benfrancis commented Aug 29, 2023

This may be an async race condition where refreshThings() in things.js intermittently gets called with an empty Map for the things argument - or at least it reports that it's empty at the point at which its size is read.

I couldn't make much sense of this using breakpoints in a debugger, but if I console.dir(things) I also sometimes get a Map which is populated with entries, but Map.size is reported as 0.

Screenshot from 2023-08-29 18-18-43

Edit: Chrome's debugger makes more sense of it, and does actually show refreshThings() being called with an empty Map for the things argument.

Screenshot from 2023-08-29 18-42-00

@benfrancis
Copy link
Member

I think what is happening here is that ThingsScreen.refreshThings() is getting called before the GatewayModel.things Map gets populated on startup. ThingsScreen.showThings() calls GatewayModel.subscribe() with the immediate argument set to true and the callback set to ThingsScreen.refreshThings(), but GatewayModel often hasn't yet had the chance to populate its collection of things by the time the callback is called.

I can fix this by removing the immediate=true argument from the call to GatewayModel.subscribe() in line 148 of ThingsScreen.showThings() (since ThingsScreen.refreshThings() is called twice more on startup after this anyway), but I'm not sure what side effects that may have.

@benfrancis
Copy link
Member

Currently I think ThingsScreen.refreshThings() is getting called 3 times on page load of /things:

  1. When ThingsScreen initially subscribes to the refreshThings event with the immediate argument set to true (at which point the things map is not always yet populated)
  2. When the /things WebSocket is successfully opened
  3. When the /groups WebSocket is successfully opened (see Things get rendered twice #3103)

I assume the intention of setting the immediate argument is to make sure that every time ThingsScreen.showThings() is called it has the latest list of things. I'm not sure this is actually necessary though because it seems to be kept in sync in the background even if the user switches to another screen. It may therefore be safe to remove the immediate argument.

@benfrancis
Copy link
Member

I think I've confirmed that ThingsScreen.showThings() is telling GatewayModel.subscribe() to immediately call back ThingsScreen.refreshThings() with the contents of GatwayModel.things, before GatewayModel.refreshThings() has had the chance to asynchronously populate the GatewayModel.things Map with a list of Things from the server.

If we wait for the WebSocket to open first, then GatewayModel.refreshThings() will be called and asynchronously call back ThingsScreen.refreshThings() as a handler once the list has been populated. So #3108 should fix the issue.

As long as there are no subsequent cases where it's important that ThingsScreen.showThings() immediately refreshes the list of Things (e.g. when switching screens), then I think we should be OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants