Skip to content

Commit 175d3bf

Browse files
bobleerbobleer
authored
Merge pull request #3340 from bobleer/bob/polish-remote-connect-dialog
refactor(web-ui): unify remote connection dialog layouts
2 parents 97bb81e + 8cd54c7 commit 175d3bf

12 files changed

Lines changed: 1838 additions & 1620 deletions

‎src/web-ui/src/app/components/RemoteConnectDialog/AccountPanel.scss‎

Lines changed: 187 additions & 120 deletions
Large diffs are not rendered by default.

‎src/web-ui/src/app/components/RemoteConnectDialog/AccountPanel.test.tsx‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ it('does not offer an update check in a runtime that cannot install updates', as
152152
await act(async () => { root.render(<AccountPanel onCloseDialog={() => {}} />); });
153153
expect(container.textContent).toContain('accountLogin.relayFailureVersionRetired');
154154
expect(container.textContent).not.toContain('update.checkForUpdates');
155+
// Without a banner action the section heading still offers a way forward.
156+
const retryButtons = Array.from(container.querySelectorAll('button'))
157+
.filter(node => node.textContent === 'accountLogin.retryConnect');
158+
expect(retryButtons).toHaveLength(1);
155159
});
156160

157161
it('offers a retry when the Relay is temporarily unavailable', async () => {
@@ -161,7 +165,10 @@ it('offers a retry when the Relay is temporarily unavailable', async () => {
161165
await act(async () => { root.render(<AccountPanel onCloseDialog={() => {}} />); });
162166
expect(container.textContent).toContain('accountLogin.relayFailureUnavailable');
163167
expect(container.textContent).not.toContain('502');
164-
expect(container.textContent).toContain('accountLogin.retryConnect');
168+
const retryButtons = Array.from(container.querySelectorAll('button'))
169+
.filter(node => node.textContent === 'accountLogin.retryConnect');
170+
expect(retryButtons).toHaveLength(1);
171+
expect(retryButtons[0]!.closest('[data-openbitfun-part="error"]')).not.toBeNull();
165172
});
166173

167174
async function retryConnection() {
@@ -441,3 +448,34 @@ it('still allows switching to a device whose compatibility flag is absent', asyn
441448
await act(async () => { peer!.click(); });
442449
expect(mocks.switchToDevice).toHaveBeenCalledWith('peer', 'Peer build');
443450
});
451+
452+
it('groups the account and its devices into titled sections with fixed row columns', async () => {
453+
mocks.accountConnectDevices.mockResolvedValue([]);
454+
mocks.getDeviceInfo.mockReset().mockResolvedValue({ device_id: 'local' });
455+
mocks.accountListDevices.mockResolvedValue([
456+
{ device_id: 'local', device_name: 'My computer', online: true },
457+
{ device_id: 'peer', device_name: 'Peer build', online: false },
458+
]);
459+
await act(async () => { root.render(<AccountPanel onCloseDialog={() => {}} />); });
460+
const titles = Array.from(container.querySelectorAll('.account-panel__section-title')).map(node => node.textContent);
461+
expect(titles).toEqual(['accountLogin.accountSectionTitle', 'accountLogin.linkedDevices']);
462+
const statuses = Array.from(container.querySelectorAll('.account-panel__device-status'))
463+
.map(node => node.getAttribute('data-openbitfun-state'));
464+
expect(statuses).toEqual(['online', 'offline']);
465+
for (const row of container.querySelectorAll('[data-openbitfun-part="deviceCard"]')) {
466+
const children = Array.from(row.children).map(node => node.className);
467+
expect(children).toEqual([
468+
'account-panel__device-select',
469+
'account-panel__device-actions',
470+
'account-panel__device-chevron',
471+
]);
472+
}
473+
});
474+
475+
it('shows the sign-in action inside the account section when signed out', async () => {
476+
mocks.identity = { resolved: true, status: 'signed-out', me: null };
477+
mocks.accountStatus.mockReset().mockResolvedValue({ logged_in: false });
478+
await act(async () => { root.render(<AccountPanel onCloseDialog={() => {}} />); });
479+
expect(container.querySelector('.account-panel__section-title')?.textContent).toBe('accountLogin.accountSectionTitle');
480+
expect(container.querySelector('[data-openbitfun-part="form"]')?.textContent).toContain('accountLogin.login');
481+
});

‎src/web-ui/src/app/components/RemoteConnectDialog/AccountPanel.tsx‎

Lines changed: 82 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { useI18n } from '@/infrastructure/i18n';
77
import {
88
confirmDanger,
99
} from '@/infrastructure/confirm-dialog';
10-
import { LogIn, Pencil, Check, X } from 'lucide-react';
10+
import { LogIn, LogOut, Pencil, Check, X } from 'lucide-react';
1111
import { DeviceSystemGlyph } from '../NavPanel/components/DeviceSystemGlyph';
1212
import { reportedHostKind } from '../NavPanel/deviceInterconnectionOverview';
1313
import { remoteConnectAPI, deviceDisplayName, deviceMetadataLabel } from '@/infrastructure/api/service-api/RemoteConnectAPI';
@@ -138,16 +138,23 @@ interface FailureBannerProps {
138138
onAction: (action: RelayFailureAction) => void;
139139
}
140140

141+
/**
142+
* The button a failure banner offers, if any. An update check only exists where
143+
* updates can be installed; elsewhere the banner stays a statement instead of
144+
* offering an action that cannot run.
145+
*/
146+
const failureBannerAction = (failure: PanelFailure): RelayFailureAction | null => (
147+
failure.action === 'retry' || (failure.action === 'check-updates' && canCheckForAppUpdates())
148+
? failure.action
149+
: null
150+
);
151+
141152
/**
142153
* Red banner plus the single next step for a classified failure. A failure whose
143154
* action is the existing sign-in flow renders no extra button.
144155
*/
145156
const FailureBanner: React.FC<FailureBannerProps> = ({ failure, t, busy, onClose, onAction }) => {
146-
// An update check only exists where updates can be installed; elsewhere the
147-
// banner stays a statement instead of offering an action that cannot run.
148-
const actionable = failure.action === 'retry' || (failure.action === 'check-updates' && canCheckForAppUpdates())
149-
? failure.action
150-
: null;
157+
const actionable = failureBannerAction(failure);
151158
return (
152159
<div className="account-panel__error-banner" data-openbitfun-component="remote-account-panel" data-openbitfun-part="error">
153160
<Alert
@@ -803,6 +810,13 @@ export const AccountPanel: React.FC<AccountPanelProps> = ({
803810
}
804811
}, [handleCheckForUpdates, handleLogin, handleRetryConnect, view]);
805812

813+
const renderSectionHeading = (id: string, title: string, action?: React.ReactNode) => (
814+
<div className="account-panel__section-heading">
815+
<h3 id={id} className="account-panel__section-title">{title}</h3>
816+
{action}
817+
</div>
818+
);
819+
806820
return (
807821
<>
808822
<div data-openbitfun-component="remote-account-panel" data-openbitfun-part="root" data-openbitfun-view={view} className="account-panel">
@@ -824,63 +838,73 @@ export const AccountPanel: React.FC<AccountPanelProps> = ({
824838

825839
{loading && view === 'devices' && (
826840
<div className="account-panel__loading-overlay" data-openbitfun-component="remote-account-panel" data-openbitfun-part="loading">
827-
<Icon name="refresh" size="lg" className="spinning" style={{ width: 20, height: 20 }} />
841+
<Icon name="refresh" size="lg" className="spinning" />
828842
<span>{t('accountLogin.processing')}</span>
829843
</div>
830844
)}
831845

832846
{view === 'login' && (
833847
<ScrollArea className="account-panel__scroll" data-openbitfun-component="remote-account-panel" data-openbitfun-part="scroll">
834-
<div className="account-panel__login-card" data-openbitfun-component="remote-account-panel" data-openbitfun-part="form">
835-
<span className="account-panel__login-icon" aria-hidden="true"><Icon name="user" size="lg" /></span>
836-
<p className="account-panel__value-prop">{t('accountLogin.loginValueProp')}</p>
837-
<p className="account-panel__security-note">{t('accountLogin.securityNote')}</p>
838-
<div className="account-panel__actions" data-openbitfun-component="remote-account-panel" data-openbitfun-part="actions">
839-
<Button variant="primary" size="sm" leadingIcon={<LogIn />} onClick={handleLogin} loading={loading && identity.status !== 'authorizing'}>
840-
{identity.status === 'authorizing' ? t('accountLogin.reopen') : loading ? t('accountLogin.processing') : t('accountLogin.login')}
841-
</Button>
848+
<section className="account-panel__section" aria-labelledby="account-panel-account-title">
849+
{renderSectionHeading('account-panel-account-title', t('accountLogin.accountSectionTitle'))}
850+
<div className="account-panel__card account-panel__login-card" data-openbitfun-component="remote-account-panel" data-openbitfun-part="form">
851+
<span className="account-panel__login-icon" aria-hidden="true"><Icon name="user" size="md" /></span>
852+
<span className="account-panel__login-copy">
853+
<span className="account-panel__value-prop">{t('accountLogin.loginValueProp')}</span>
854+
<span className="account-panel__security-note">{t('accountLogin.securityNote')}</span>
855+
</span>
856+
<div className="account-panel__actions" data-openbitfun-component="remote-account-panel" data-openbitfun-part="actions">
857+
<Button variant="primary" size="sm" leadingIcon={<LogIn size={14} />} onClick={handleLogin} loading={loading && identity.status !== 'authorizing'}>
858+
{identity.status === 'authorizing' ? t('accountLogin.reopen') : loading ? t('accountLogin.processing') : t('accountLogin.login')}
859+
</Button>
860+
</div>
842861
</div>
843-
</div>
862+
</section>
844863
</ScrollArea>
845864
)}
846865

847866
{view === 'devices' && (
848867
<ScrollArea className="account-panel__scroll" data-openbitfun-component="remote-account-panel" data-openbitfun-part="scroll">
849-
<div className="account-panel__identity-line">
850-
<Avatar key={username} size="md" src={identity.me?.user.avatarUrl} alt={username} aria-label={username}>
851-
{username.trim().charAt(0).toUpperCase() || <Icon name="user" />}
852-
</Avatar>
853-
<span className="account-panel__identity-copy">
854-
<span className="account-panel__identity-label">{t('accountLogin.signedInAccount')}</span>
855-
<OverflowText className="account-panel__identity-name" title={username}>{username.trim()}</OverflowText>
856-
</span>
857-
<Button variant="text" size="sm" onClick={handleLogout} disabled={loading}>
858-
{t('accountLogin.logout')}
859-
</Button>
860-
</div>
861-
<div className="account-panel__section-heading">
862-
<h3>{t('accountLogin.linkedDevices')}</h3>
863-
<Button variant="text" size="sm" leadingIcon={<Icon name="refresh" size="sm" />}
864-
onClick={relayFailure ? handleRetryConnect : refreshDevices} disabled={loading}>
865-
{t(relayFailure ? 'accountLogin.retryConnect' : 'accountLogin.refreshDevices')}
866-
</Button>
867-
</div>
868-
<div className="account-panel__devices-card">
868+
<section className="account-panel__section" aria-labelledby="account-panel-account-title">
869+
{renderSectionHeading('account-panel-account-title', t('accountLogin.accountSectionTitle'))}
870+
<div className="account-panel__card account-panel__identity-line">
871+
<Avatar key={username} size="md" src={identity.me?.user.avatarUrl} alt={username} aria-label={username}>
872+
{username.trim().charAt(0).toUpperCase() || <Icon name="user" />}
873+
</Avatar>
874+
<span className="account-panel__identity-copy">
875+
<OverflowText className="account-panel__identity-name" title={username}>{username.trim()}</OverflowText>
876+
<span className="account-panel__identity-label">{t('accountLogin.signedInAccount')}</span>
877+
</span>
878+
<Button variant="outline" size="sm" leadingIcon={<LogOut size={14} />} onClick={handleLogout} disabled={loading}>
879+
{t('accountLogin.logout')}
880+
</Button>
881+
</div>
882+
</section>
883+
<section className="account-panel__section" aria-labelledby="account-panel-devices-title">
884+
{renderSectionHeading('account-panel-devices-title', t('accountLogin.linkedDevices'), (
885+
relayFailure && failureBannerAction(relayFailure) ? null : (
886+
<Button variant="text" size="xs" leadingIcon={<Icon name="refresh" size="sm" />}
887+
onClick={relayFailure ? handleRetryConnect : refreshDevices} disabled={loading}>
888+
{t(relayFailure ? 'accountLogin.retryConnect' : 'accountLogin.refreshDevices')}
889+
</Button>
890+
)
891+
))}
869892
{aliasCapability === 'unsupported' && <Alert tone="info" message={t('accountLogin.deviceAliasUnsupported')} />}
870893
{relayFailure && (
871894
<FailureBanner failure={relayFailure} t={t} busy={loading} onAction={runFailureAction} />
872895
)}
873-
<div className="account-panel__device-list" data-openbitfun-component="remote-account-panel" data-openbitfun-part="deviceList">
874-
{!relayFailure && devicesReady && devices.length === 0 && (
875-
<div className="account-panel__empty">{t('accountLogin.noDevices')}</div>
876-
)}
877-
{!relayFailure && !devicesReady && (
878-
<div className="account-panel__empty account-panel__empty--loading" role="status">
879-
<Icon name="refresh" size="sm" className="spinning" />
880-
{t('accountLogin.loadingDevices')}
881-
</div>
882-
)}
883-
{!relayFailure && sortedDevices.map((d) => {
896+
{!relayFailure && <div className="account-panel__card account-panel__devices-card">
897+
<div className="account-panel__device-list" data-openbitfun-component="remote-account-panel" data-openbitfun-part="deviceList">
898+
{devicesReady && devices.length === 0 && (
899+
<div className="account-panel__empty">{t('accountLogin.noDevices')}</div>
900+
)}
901+
{!devicesReady && (
902+
<div className="account-panel__empty account-panel__empty--loading" role="status">
903+
<Icon name="refresh" size="sm" className="spinning" />
904+
{t('accountLogin.loadingDevices')}
905+
</div>
906+
)}
907+
{sortedDevices.map((d) => {
884908
const isLocal = localDeviceId === d.device_id;
885909
// This machine is selectable while the window renders a peer,
886910
// so the dialog can bring the UI back without disconnecting.
@@ -927,7 +951,9 @@ export const AccountPanel: React.FC<AccountPanelProps> = ({
927951
'aria-label': t('accountLogin.openDevice', { name: displayName }),
928952
} : {})}
929953
>
930-
<DeviceSystemGlyph device={systemFacts} />
954+
<span className="account-panel__device-glyph" aria-hidden="true">
955+
<DeviceSystemGlyph device={systemFacts} size={18} />
956+
</span>
931957
<span className="account-panel__device-info">
932958
{/* Renaming replaces the name in place: showing the old
933959
name next to an editor reads as two device names. */}
@@ -986,12 +1012,11 @@ export const AccountPanel: React.FC<AccountPanelProps> = ({
9861012
</span>
9871013
)}
9881014
</span>
989-
{isSelectable && <Icon name="chevron-right" size="sm" />}
9901015
</DeviceEntry>
9911016
{/* The open editor owns the row's actions; keeping rename and
9921017
delete beside it rendered two competing icon clusters. */}
9931018
{editingDeviceId !== d.device_id && (
994-
<>
1019+
<span className="account-panel__device-actions">
9951020
<IconButton
9961021
aria-label={t('accountLogin.editDeviceAlias')}
9971022
disabled={loading || editingDeviceId !== null || !aliasSupported || savingAliasId !== null}
@@ -1010,13 +1035,17 @@ export const AccountPanel: React.FC<AccountPanelProps> = ({
10101035
title={removeLabel}
10111036
variant="quiet"
10121037
/>
1013-
</>
1038+
</span>
10141039
)}
1040+
<span className="account-panel__device-chevron" aria-hidden="true">
1041+
{isSelectable && editingDeviceId !== d.device_id && <Icon name="chevron-right" size="sm" />}
1042+
</span>
10151043
</div>
10161044
);
10171045
})}
1018-
</div>
1019-
</div>
1046+
</div>
1047+
</div>}
1048+
</section>
10201049
</ScrollArea>
10211050
)}
10221051
</div>

0 commit comments

Comments
 (0)