Skip to content

Handling tab visibility in getNextConnectionState #469

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 12, 2024
Merged

Handling tab visibility in getNextConnectionState #469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
04991d2
Only ignore status updates when on radio and continue updating status…
microbit-grace Nov 5, 2024
99ccd77
Merge branch 'main' of https://github.com/microbit-foundation/ml-trai…
microbit-grace Nov 8, 2024
a4f1141
Handle tab visibility in getNextConnectionState
microbit-grace Nov 11, 2024
7418310
Revert unnecessary changes
microbit-grace Nov 11, 2024
31ad9a0
Revert unnecessary changes
microbit-grace Nov 11, 2024
0475af0
Add comment
microbit-grace Nov 11, 2024
6ce3aad
Revert unnecessary changes
microbit-grace Nov 11, 2024
148d9a8
Tweaks
microbit-grace Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/connect-status-hooks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,6 @@ export const useConnectStatusUpdater = (
useState<boolean>(true);

useEffect(() => {
if (
!isBrowserTabVisible &&
currConnType === "radio" &&
connectionStatus === ConnectionStatus.Connected
) {
// Show reconnecting when user hides browser tab and is radio connected
setConnectionStatus(ConnectionStatus.ReconnectingAutomatically);
return;
}
const listener: StatusListener = ({ status: deviceStatus, type }) => {
const nextState = getNextConnectionState({
currConnType,
Expand All @@ -133,9 +124,10 @@ export const useConnectStatusUpdater = (
setHasAttemptedReconnect,
onFirstConnectAttempt,
setOnFirstConnectAttempt,
isBrowserTabVisible,
});
prevDeviceStatus.current = deviceStatus;
if (nextState && isBrowserTabVisible) {
if (nextState) {
handleStatus && handleStatus(nextState.status, nextState.flowType);
setConnectionStatus(nextState.status);
}
Expand Down
31 changes: 27 additions & 4 deletions src/get-next-connection-state.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ const testGetNextConnectionState = ({
expectedNextConnectionState,
expectedHasAttemptedReconnect,
expectedOnFirstConnectAttempt,
isBrowserTabVisible,
}: {
input: Input;
initialHasAttemptedReconnect: boolean;
expectedNextConnectionState: NextConnectionState;
expectedHasAttemptedReconnect: boolean;
initialOnFirstConnectAttempt: boolean;
expectedOnFirstConnectAttempt: boolean;
isBrowserTabVisible?: boolean;
}) => {
let hasAttempedReconnect = initialHasAttemptedReconnect;
let onFirstConnectAttempt = initialOnFirstConnectAttempt;
Expand All @@ -48,6 +50,7 @@ const testGetNextConnectionState = ({
setHasAttemptedReconnect: (val: boolean) => {
hasAttempedReconnect = val;
},
isBrowserTabVisible: isBrowserTabVisible ?? true,
});
expect(result).toEqual(expectedNextConnectionState);
expect(hasAttempedReconnect).toEqual(expectedHasAttemptedReconnect);
Expand Down Expand Up @@ -154,7 +157,7 @@ describe("getNextConnectionState for radio connection", () => {
type: "usb",
},
initialOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: true,
initialHasAttemptedReconnect: false,
expectedHasAttemptedReconnect: false,
expectedNextConnectionState: undefined,
Expand Down Expand Up @@ -233,6 +236,26 @@ describe("getNextConnectionState for radio connection", () => {
},
});
});
test("radio bridge device showing reconnecting automatically because tab is no longer visible", () => {
testGetNextConnectionState({
input: {
currConnType: "radio",
currStatus: ConnectionStatus.Connected,
deviceStatus: DeviceConnectionStatus.DISCONNECTED,
prevDeviceStatus: DeviceConnectionStatus.NO_AUTHORIZED_DEVICE,
type: "usb",
},
initialOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: false,
initialHasAttemptedReconnect: false,
isBrowserTabVisible: false,
expectedHasAttemptedReconnect: false,
expectedNextConnectionState: {
status: ConnectionStatus.ReconnectingAutomatically,
flowType: ConnectionFlowType.ConnectRadioBridge,
},
});
});
test("radio bridge device reconnect fail", () => {
testGetNextConnectionState({
input: {
Expand Down Expand Up @@ -264,7 +287,7 @@ describe("getNextConnectionState for radio connection", () => {
initialOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: false,
initialHasAttemptedReconnect: true,
expectedHasAttemptedReconnect: false,
expectedHasAttemptedReconnect: true,
expectedNextConnectionState: {
status: ConnectionStatus.FailedToReconnectTwice,
flowType: ConnectionFlowType.ConnectRadioRemote,
Expand Down Expand Up @@ -321,7 +344,7 @@ describe("getNextConnectionState for radio connection", () => {
initialOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: false,
initialHasAttemptedReconnect: true,
expectedHasAttemptedReconnect: false,
expectedHasAttemptedReconnect: true,
expectedNextConnectionState: {
status: ConnectionStatus.FailedToReconnectTwice,
flowType: ConnectionFlowType.ConnectRadioRemote,
Expand Down Expand Up @@ -527,7 +550,7 @@ describe("getNextConnectionState for bluetooth connection", () => {
initialOnFirstConnectAttempt: false,
expectedOnFirstConnectAttempt: false,
initialHasAttemptedReconnect: true,
expectedHasAttemptedReconnect: false,
expectedHasAttemptedReconnect: true,
expectedNextConnectionState: {
status: ConnectionStatus.FailedToReconnectTwice,
flowType: ConnectionFlowType.ConnectBluetooth,
Expand Down
38 changes: 27 additions & 11 deletions src/get-next-connection-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface GetNextConnectionStateInput {
setHasAttemptedReconnect: (val: boolean) => void;
onFirstConnectAttempt: boolean;
setOnFirstConnectAttempt: (val: boolean) => void;
isBrowserTabVisible: boolean;
}

export type NextConnectionState =
Expand All @@ -29,6 +30,7 @@ export const getNextConnectionState = ({
setHasAttemptedReconnect,
onFirstConnectAttempt,
setOnFirstConnectAttempt,
isBrowserTabVisible,
}: GetNextConnectionStateInput): NextConnectionState => {
if (currStatus === ConnectionStatus.Disconnected) {
// Do not update connection status when user explicitly disconnected connection
Expand All @@ -42,6 +44,27 @@ export const getNextConnectionState = ({
? ConnectionFlowType.ConnectRadioRemote
: ConnectionFlowType.ConnectBluetooth;

// Don't update connection status when hiding browser tab if connection
// status is already set to an error case.
if (
!isBrowserTabVisible &&
(currStatus === ConnectionStatus.ConnectionLost ||
currStatus === ConnectionStatus.FailedToReconnect ||
currStatus === ConnectionStatus.FailedToReconnectTwice)
) {
return undefined;
}

const hasStartedOver =
currStatus === ConnectionStatus.NotConnected ||
currStatus === ConnectionStatus.FailedToConnect ||
currStatus === ConnectionStatus.FailedToReconnectTwice;

if (hasStartedOver) {
setHasAttemptedReconnect(false);
setOnFirstConnectAttempt(true);
}

// We use usb status to infer the radio bridge device status for handling error.
if (type === "usb") {
if (
Expand All @@ -59,11 +82,14 @@ export const getNextConnectionState = ({
) {
return undefined;
}
// Show reconnecting automatically when user hides browser tab and there is a connection error.
if (!isBrowserTabVisible) {
return { status: ConnectionStatus.ReconnectingAutomatically, flowType };
}
if (
// If bridge micro:bit causes radio bridge reconnect to fail twice
hasAttempedReconnect
) {
setHasAttemptedReconnect(false);
Copy link
Author

@microbit-grace microbit-grace Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Letting hasStartedOver logic handle the resetting of this instead for when reconnect fail twice. Also, important to setOnFirstConnectAttempt(true) for attempts after reconnect fail twice to avoid reconnect fail twice error dialog from being triggered again in subsequent start over connection attempt errors

return {
status: ConnectionStatus.FailedToReconnectTwice,
flowType: ConnectionFlowType.ConnectRadioRemote,
Expand All @@ -78,15 +104,6 @@ export const getNextConnectionState = ({
return { status, flowType };
}

const hasStartedOver =
currStatus === ConnectionStatus.NotConnected ||
currStatus === ConnectionStatus.FailedToConnect;

if (hasStartedOver) {
setHasAttemptedReconnect(false);
setOnFirstConnectAttempt(true);
}

if (
// If user starts or restarts connection flow.
// Disconnection happens for newly started / restarted
Expand Down Expand Up @@ -126,7 +143,6 @@ export const getNextConnectionState = ({
hasAttempedReconnect &&
deviceStatus === DeviceConnectionStatus.DISCONNECTED
) {
setHasAttemptedReconnect(false);
return { status: ConnectionStatus.FailedToReconnectTwice, flowType };
}
if (
Expand Down
Loading