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

[WIP] Fix button saying server offline after it comes back. #3213

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e37583c
Fix button saying server offline after it comes back.
psybers Aug 7, 2024
7511636
add release note
psybers Aug 7, 2024
3c65fb1
Merge branch 'master' into server-offline
psybers Aug 7, 2024
e98bd97
Merge branch 'master' into server-offline
psybers Aug 9, 2024
f0efcdb
Merge branch 'master' into server-offline
psybers Aug 10, 2024
03e5967
Merge branch 'master' into server-offline
psybers Aug 12, 2024
2c3697c
Merge branch 'master' into server-offline
psybers Aug 13, 2024
29d98c9
Merge branch 'master' into server-offline
psybers Aug 14, 2024
407df34
Merge branch 'master' into server-offline
psybers Aug 14, 2024
8f8f1be
Merge branch 'master' into server-offline
psybers Aug 20, 2024
843d53d
Merge branch 'master' into server-offline
psybers Aug 21, 2024
5a1c1a5
Merge branch 'master' into server-offline
psybers Aug 29, 2024
eece002
Merge branch 'master' into server-offline
psybers Sep 2, 2024
dcc4982
Merge branch 'master' into server-offline
psybers Sep 10, 2024
f72c132
Merge branch 'master' into server-offline
psybers Sep 19, 2024
372dc3d
Merge branch 'master' into server-offline
psybers Sep 20, 2024
ec749a0
Merge branch 'master' into server-offline
psybers Sep 28, 2024
7db558a
Merge branch 'master' into server-offline
psybers Oct 4, 2024
5de0e54
Merge branch 'master' into server-offline
psybers Oct 5, 2024
4f145a1
Merge branch 'master' into server-offline
psybers Oct 14, 2024
d325722
Merge branch 'master' into server-offline
psybers Oct 22, 2024
d595eb3
Merge branch 'master' into server-offline
psybers Oct 23, 2024
0adbeb6
Merge branch 'master' into server-offline
psybers Oct 30, 2024
2b0f5b9
Merge branch 'master' into server-offline
psybers Nov 8, 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
18 changes: 8 additions & 10 deletions packages/desktop-client/src/components/LoggedInUser.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
// @ts-strict-ignore
import React, { useState, useEffect, useRef } from 'react';
import { useSelector } from 'react-redux';

import { type State } from 'loot-core/src/client/state-types';
import React, { useState, useEffect, useRef, useMemo } from 'react';

import { useActions } from '../hooks/useActions';
import { theme, styles, type CSSProperties } from '../style';
Expand All @@ -16,15 +13,16 @@ import { useServerURL } from './ServerContext';

type LoggedInUserProps = {
hideIfNoServer?: boolean;
syncState: null | 'offline' | 'local' | 'disabled' | 'error';
style?: CSSProperties;
color?: string;
};
export function LoggedInUser({
hideIfNoServer,
syncState,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. This component is also used on the ManagementApp.jsx screen - can we pass the syncState from there too?

test2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I should be able to do this there as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok so after looking into this area of the code, it definitely won't work.

This is making me rethink how that button works in general, and the approach used. Perhaps the button needs its own logic to determine the state instead of relying on other components.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is currently working in the latest master so if we merge this branch we'd be breaking it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, like I said it probably needs reworked.

style,
color,
}: LoggedInUserProps) {
const userData = useSelector((state: State) => state.user.data);
const { getUserData, signOut, closeBudget } = useActions();
const [loading, setLoading] = useState(true);
const [menuOpen, setMenuOpen] = useState(false);
Expand Down Expand Up @@ -62,17 +60,17 @@ export function LoggedInUser({
}
}

function serverMessage() {
const serverMessage = useMemo(() => {
if (!serverUrl) {
return 'No server';
}

if (userData?.offline) {
if (syncState === 'offline') {
return 'Server offline';
}

return 'Server online';
}
}, [serverUrl, syncState]);

if (hideIfNoServer && !serverUrl) {
return null;
Expand Down Expand Up @@ -101,7 +99,7 @@ export function LoggedInUser({
onPress={() => setMenuOpen(true)}
style={color && { color }}
>
{serverMessage()}
{serverMessage}
</Button>

<Popover
Expand All @@ -113,7 +111,7 @@ export function LoggedInUser({
onMenuSelect={onMenuSelect}
items={[
serverUrl &&
!userData?.offline && {
syncState === null && {
name: 'change-password',
text: 'Change password',
},
Expand Down
29 changes: 22 additions & 7 deletions packages/desktop-client/src/components/Titlebar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect } from 'react';
import React, { useState, useEffect, type SetStateAction } from 'react';
import { useHotkeys } from 'react-hotkeys-hook';
import { Routes, Route, useLocation } from 'react-router-dom';

Expand Down Expand Up @@ -96,16 +96,22 @@ function PrivacyButton({ style }: PrivacyButtonProps) {

type SyncButtonProps = {
style?: CSSProperties;
syncState: string | null;
setSyncState: (
state: SetStateAction<null | 'offline' | 'local' | 'disabled' | 'error'>,
) => void;
isMobile?: boolean;
};
function SyncButton({ style, isMobile = false }: SyncButtonProps) {
function SyncButton({
style,
syncState,
setSyncState,
isMobile = false,
}: SyncButtonProps) {
const [cloudFileId] = useMetadataPref('cloudFileId');
const { sync } = useActions();

const [syncing, setSyncing] = useState(false);
const [syncState, setSyncState] = useState<
null | 'offline' | 'local' | 'disabled' | 'error'
>(null);

useEffect(() => {
const unlisten = listen('sync-event', ({ type, subtype, syncDisabled }) => {
Expand Down Expand Up @@ -263,6 +269,9 @@ export function Titlebar({ style }: TitlebarProps) {
const { isNarrowWidth } = useResponsive();
const serverURL = useServerURL();
const [floatingSidebar] = useGlobalPref('floatingSidebar');
const [syncState, setSyncState] = useState<
null | 'offline' | 'local' | 'disabled' | 'error'
>(null);

return isNarrowWidth ? null : (
<View
Expand Down Expand Up @@ -333,8 +342,14 @@ export function Titlebar({ style }: TitlebarProps) {
<ThemeSelector style={{ marginLeft: 10 }} />
)}
<PrivacyButton style={{ marginLeft: 10 }} />
{serverURL ? <SyncButton style={{ marginLeft: 10 }} /> : null}
<LoggedInUser style={{ marginLeft: 10 }} />
{serverURL ? (
<SyncButton
syncState={syncState}
setSyncState={setSyncState}
style={{ marginLeft: 10 }}
/>
) : null}
<LoggedInUser syncState={syncState} style={{ marginLeft: 10 }} />
</View>
);
}
6 changes: 6 additions & 0 deletions upcoming-release-notes/3213.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Bugfix
authors: [psybers]
---

Fix button saying server offline after it comes back.
Loading