LivePage: refactor LivePage and make changedShortIds a derived #3208
Conversation
|
Build for latest commit 46e81a9 is at https://pr3208.build.csb.dev/s/new. |
|
Okay, so I changed how we manage signing into a Live session, when not signed in with a profile. The Instead I created a new action for signing in to a Live session. This logs the user in first. |
|
|
||
| export class BlinkingDot extends React.PureComponent { | ||
| export class BlinkingDot extends React.PureComponent<{}, { showing: boolean }> { | ||
| timer: NodeJS.Timer; |
There was a problem hiding this comment.
I believe in the browser case we need number here. Then use this.timer = window.setInterval.
There was a problem hiding this comment.
Not blocking from merging, but for reference later.
There was a problem hiding this comment.
Ah, yeah, hm... weird typing behaviour. Was not aware that window.setInterval and setInterval was typed differently. Fixed
CompuIves
left a comment
CompuIves
left a comment
There was a problem hiding this comment.
This looks good! Only a few questions left, especially the one related the live.isLoading.
|
During testing: when joining a live session I initially see the sign in button, then the loading live. Is there a way to not show the button? I think we can use |
Okay, now it is flipped. It shows "joining..." and then if not authenticated it shows the button instead of continuing into the Live session :) |
Okay, so the
LivePagehas been tested here and should work now. Related to looking into this I saw that we should rather derivechangedShortIds. It is not being used much now, but it allowed us to remove a bunch of code. This change is not very well tested, not sure about all scenarios.I also noticed an issue with unsaved code and changing sandbox. The code is saved nevertheless, and in LIVE mode it causes an error. We need to fix this, but should be a different PR 😄