Improve Connect My Computer UI & logout experience (#32786)

* Kill agent after removing workspace on logout

Quick shutdown of an agent now induces a 3 second timeout if there are
active connections (#31869). I changed the logout procedure so that we
first remove the workspace (and thus close all tabs) and only then kill
the agent. This makes it so that if there were any open connections from
the app, we'll close them before killing the agent, which means the app
will close without that 3 second timeout.

* Make Start Agent button's handler ignore errors

* Avoid rendering labels if there's no node

This could happen if someone started and stopped the agent and then clicked
Start Agent with expired certs. agentNode would go from undefined to defined
back to undefined, but it seems that <Transition> doesn't unmount immediately.

* clusters.Cluster.Logout: Make error handling more explicit

!trace.IsNotFound(err) returns true both when the error is nil and when
it's not nil but the error is not NotFound. Meaning that code after the
conditional would run only when err is NotFound.

I added a bogus error to test something after the conditional and then
spent 30 minutes wondering what's going on.

* Display an empty ring if agent is set up but not running

This makes it consistent with the Connections list which also displays
an empty ring if there are no connections.

* Expand NavigationMenu stories, add story for agent starting

* Avoid calculating indicator status if user cannot use feature

* Copy progress icon from Cloud's ProgressBar

* Return 'not-configured' early if agent is not configured

* Add story for when agent is configured but not started, reorder stories
This commit is contained in:
Rafał Cieślak 2023-09-29 14:55:58 +02:00 committed by GitHub
parent 818d9edb2a
commit 8098f63d2f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 201 additions and 94 deletions

View file

@ -71,7 +71,7 @@ func (c *Cluster) Logout(ctx context.Context) error {
}
// Remove keys for this user from disk and running agent.
if err := c.clusterClient.Logout(); !trace.IsNotFound(err) {
if err := c.clusterClient.Logout(); err != nil && !trace.IsNotFound(err) {
return trace.Wrap(err)
}

View file

@ -27,7 +27,6 @@ export function useClusterLogout({
}) {
const ctx = useAppContext();
const [{ status, statusText }, removeCluster] = useAsync(async () => {
await ctx.connectMyComputerService.killAgentAndRemoveData(clusterUri);
await ctx.clustersService.logout(clusterUri);
if (ctx.workspacesService.getRootClusterUri() === clusterUri) {
@ -40,11 +39,20 @@ export function useClusterLogout({
}
}
// remove connections first, they depend both on the cluster and the workspace
// Remove connections first, they depend both on the cluster and the workspace.
ctx.connectionTracker.removeItemsBelongingToRootCluster(clusterUri);
// remove the workspace next, because it depends on the cluster
// Remove the workspace next, because it depends on the cluster.
ctx.workspacesService.removeWorkspace(clusterUri);
// remove the cluster, it does not depend on anything
// If there are active ssh connections to the agent, killing it will take a few seconds. To work
// around this, kill the agent only after removing the workspace. Removing the workspace closes
// ssh tabs, so it should terminate connections to the cluster from the app.
//
// If ClustersService.logout above fails, the user should still be able to manage the agent.
await ctx.connectMyComputerService.killAgentAndRemoveData(clusterUri);
// Remove the cluster, it does not depend on anything.
await ctx.clustersService.removeClusterAndResources(clusterUri);
});

View file

@ -17,7 +17,7 @@
import React from 'react';
import styled, { useTheme } from 'styled-components';
import { Flex, Box } from 'design';
import { Check, Warning, Refresh } from 'design/Icon';
import * as icons from 'design/Icon';
import { decomposeColor, emphasize } from 'design/theme/utils/colorManipulator';
import { AttemptStatus } from 'shared/hooks/useAsync';
@ -74,7 +74,13 @@ function Phase({
return (
<>
<StyledPhase mr="3" bg={bg}>
<StyledPhase
mr="3"
bg={bg}
css={`
position: relative;
`}
>
<PhaseIcon status={status} />
</StyledPhase>
{!isLast && (
@ -108,29 +114,19 @@ const StyledLine = styled(Box)`
function PhaseIcon({ status }: { status: AttemptStatus }): JSX.Element {
if (status === 'success') {
return <Check size="small" color="white" />;
return <icons.Check size="small" color="white" />;
}
if (status === 'error') {
return <Warning size="small" color="white" />;
return <icons.Warning size="small" color="white" />;
}
if (status === 'processing') {
return (
<Refresh
size="extraLarge"
color="success"
css={`
animation: anim-rotate 1.5s infinite linear;
@keyframes anim-rotate {
0% {
transform: rotate(0);
}
100% {
transform: rotate(360deg);
}
`}
/>
<>
<Spinner />
<icons.Restore size="small" color="buttons.text" />
</>
);
}
@ -151,3 +147,23 @@ function getPhaseSolidColor(theme: any): string {
const alpha = decomposeColor(theme.colors.spotBackground[1]).values[3] || 0;
return emphasize(theme.colors.levels.surface, alpha);
}
const Spinner = styled.div`
opacity: 1;
color: ${props => props.theme.colors.spotBackground[2]};
border: 3px solid ${props => props.theme.colors.success};
border-radius: 50%;
border-top: 3px solid ${props => props.theme.colors.spotBackground[0]};
width: 24px;
height: 24px;
position: absolute;
animation: spinner 4s linear infinite;
@keyframes spinner {
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
`;

View file

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React from 'react';
import React, { useCallback } from 'react';
import {
Alert,
Box,
@ -82,6 +82,13 @@ export function DocumentConnectMyComputerStatus(
const isAgentIncompatible = agentCompatibility === 'incompatible';
const isAgentIncompatibleOrUnknown =
agentCompatibility === 'incompatible' || agentCompatibility === 'unknown';
const downloadAndStartAgentAndIgnoreErrors = useCallback(async () => {
try {
await downloadAndStartAgent();
} catch (error) {
// Ignore the error, it'll be shown in the UI by inspecting the attempts.
}
}, [downloadAndStartAgent]);
const prettyCurrentAction = prettifyCurrentAction(currentAction);
@ -222,7 +229,9 @@ export function DocumentConnectMyComputerStatus(
>
{state => (
<LabelsContainer gap={1} className={state}>
{renderLabels(agentNode.labelsList)}
{/* Explicitly check for existence of agentNode because Transition doesn't seem to
unmount immediately when `in` becomes falsy. */}
{agentNode?.labelsList && renderLabels(agentNode.labelsList)}
</LabelsContainer>
)}
</Transition>
@ -298,7 +307,7 @@ export function DocumentConnectMyComputerStatus(
<ButtonPrimary
block
disabled={disableStartAgentButton}
onClick={downloadAndStartAgent}
onClick={downloadAndStartAgentAndIgnoreErrors}
size="large"
data-testid="start-agent"
>

View file

@ -14,9 +14,7 @@
* limitations under the License.
*/
import React from 'react';
import { wait } from 'shared/utils/wait';
import React, { useEffect, useRef, useLayoutEffect } from 'react';
import { MockWorkspaceContextProvider } from 'teleterm/ui/fixtures/MockWorkspaceContextProvider';
import { makeRootCluster } from 'teleterm/services/tshd/testHelpers';
@ -34,6 +32,46 @@ export default {
title: 'Teleterm/ConnectMyComputer/NavigationMenu',
};
export function AgenNotConfigured() {
return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
return false;
}}
/>
);
}
export function AgentConfiguredButNotStarted() {
return <ShowState agentProcessState={{ status: 'not-started' }} />;
}
export function AgentStarting() {
const abortControllerRef = useRef(new AbortController());
useEffect(() => {
return () => {
abortControllerRef.current.abort();
};
}, []);
const appContext = new MockAppContext({ appVersion: '17.0.0' });
appContext.connectMyComputerService.downloadAgent = () =>
new Promise((resolve, reject) => {
abortControllerRef.current.signal.addEventListener('abort', () => reject);
});
return (
<ShowState
appContext={appContext}
agentProcessState={{ status: 'not-started' }}
autoStart={true}
/>
);
}
export function AgentRunning() {
return <ShowState agentProcessState={{ status: 'running' }} />;
}
@ -76,25 +114,24 @@ export function AgentExitedUnsuccessfully() {
);
}
export function AgentSetupNotDone() {
return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
return false;
}}
/>
);
}
export function LoadingAgentConfigFile() {
const abortControllerRef = useRef(new AbortController());
useEffect(() => {
return () => {
abortControllerRef.current.abort();
};
}, []);
const getPromiseRejectedOnUnmount = () =>
new Promise<boolean>((resolve, reject) => {
abortControllerRef.current.signal.addEventListener('abort', () => reject);
});
return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
await wait(60_000);
return true;
}}
isAgentConfigFileCreated={getPromiseRejectedOnUnmount}
/>
);
}
@ -113,14 +150,18 @@ export function FailedToLoadAgentConfigFile() {
function ShowState({
isAgentConfigFileCreated = async () => true,
agentProcessState,
appContext = new MockAppContext(),
autoStart = false,
}: {
agentProcessState: AgentProcessState;
isAgentConfigFileCreated?: () => Promise<boolean>;
appContext?: MockAppContext;
autoStart?: boolean;
}) {
const cluster = makeRootCluster({
features: { isUsageBasedBilling: true, advancedAccessWorkflows: false },
proxyVersion: '17.0.0',
});
const appContext = new MockAppContext();
appContext.clustersService.state.clusters.set(cluster.uri, cluster);
appContext.configService = createMockConfigService({
'feature.connectMyComputer': true,
@ -139,6 +180,21 @@ function ShowState({
appContext.connectMyComputerService.isAgentConfigFileCreated =
isAgentConfigFileCreated;
if (autoStart) {
appContext.workspacesService.setConnectMyComputerAutoStart(
cluster.uri,
true
);
}
useLayoutEffect(() => {
(
document.querySelector(
'[data-testid=connect-my-computer-icon]'
) as HTMLButtonElement
)?.click();
});
return (
<MockAppContextProvider appContext={appContext}>
<MockWorkspaceContextProvider rootClusterUri={cluster.uri}>

View file

@ -32,8 +32,14 @@ import {
/**
* IndicatorStatus combines a couple of different states into a single enum which dictates the
* decorative look of NavigationMenu.
*
* 'not-configured' means that the user did not interact with the feature and thus no indicator
* should be shown.
* '' means that the user has interacted with the feature, but the agent is not currently running in
* which case we display an empty circle, like we do next to the Connections icon when there's no
* active connections.
*/
type IndicatorStatus = AttemptStatus;
type IndicatorStatus = AttemptStatus | 'not-configured';
export function NavigationMenu() {
const iconRef = useRef();
@ -41,15 +47,16 @@ export function NavigationMenu() {
const { documentsService, rootClusterUri } = useWorkspaceContext();
const { isAgentConfiguredAttempt, currentAction, canUse } =
useConnectMyComputerContext();
const indicatorStatus = getIndicatorStatus(
currentAction,
isAgentConfiguredAttempt
);
if (!canUse) {
return null;
}
const indicatorStatus = getIndicatorStatus(
currentAction,
isAgentConfiguredAttempt
);
function toggleMenu() {
setIsMenuOpened(wasOpened => !wasOpened);
}
@ -117,6 +124,19 @@ function getIndicatorStatus(
if (isAgentConfiguredAttempt.status === 'error') {
return 'error';
}
const isAgentConfigured =
isAgentConfiguredAttempt.status === 'success' &&
isAgentConfiguredAttempt.data;
// Returning 'not-configured' early means that the indicator won't be shown until the user
// completes the setup.
//
// This is fine, as the setup has multiple steps but not all come from the context (and thus are
// not ever assigned to currentAction). This means that if the indicator was shown during the
// setup, it would not work reliably, as it would not reflect the progress of certain steps.
if (!isAgentConfigured) {
return 'not-configured';
}
if (currentAction.kind === 'observe-process') {
switch (currentAction.agentProcessState.status) {
@ -163,61 +183,19 @@ export const MenuIcon = forwardRef<HTMLDivElement, MenuIconProps>(
kind="secondary"
size="small"
title="Open Connect My Computer"
data-testid="connect-my-computer-icon"
>
<Laptop size="medium" />
{props.indicatorStatus === 'error' ? (
<StyledWarning />
) : (
indicatorStatusToStyledStatus(props.indicatorStatus)
<StyledStatus status={props.indicatorStatus} />
)}
</StyledButton>
);
}
);
const indicatorStatusToStyledStatus = (
indicatorStatus: '' | 'processing' | 'success'
): JSX.Element => {
return (
<StyledStatus
status={indicatorStatus}
css={`
@keyframes blink {
0% {
opacity: 0;
}
50% {
opacity: 100%;
}
100% {
opacity: 0;
}
}
animation: blink 1.4s ease-in-out;
animation-iteration-count: ${props => {
const hasFinished =
props.status === 'success' || props.status === 'error';
return hasFinished ? '0' : 'infinite';
}};
visibility: ${props => (props.status === '' ? 'hidden' : 'visible')};
background: ${props => getIndicatorColor(props.status, props.theme)};
`}
/>
);
};
function getIndicatorColor(
status: 'processing' | 'success',
theme: any
): string {
switch (status) {
case 'processing':
case 'success':
return theme.colors.success;
}
}
const StyledButton = styled(Button)`
position: relative;
background: ${props => props.theme.colors.spotBackground[0]};
@ -235,6 +213,46 @@ const StyledStatus = styled(Box)`
height: 8px;
border-radius: 50%;
box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1);
@keyframes blink {
0% {
opacity: 0;
}
50% {
opacity: 100%;
}
100% {
opacity: 0;
}
}
animation: blink 1.4s ease-in-out;
animation-iteration-count: ${props =>
props.status === 'processing' ? 'infinite' : '0'};
${props => {
const { status, theme } = props;
// 'not-configured' means that the user did not interact with the feature and thus no indicator
// should be shown.
if (status === 'not-configured') {
return { visibility: 'hidden' };
}
// '' means that the user has interacted with the feature, but the agent is not currently
// running in which case we display an empty circle, like we do next to the Connections icon
// when there's no active connections.
if (status === '') {
return {
border: `1px solid ${theme.colors.text.slightlyMuted}`,
};
}
if (status === 'processing' || status === 'success') {
return { backgroundColor: theme.colors.success };
}
// 'error' status can be ignored as it's handled outside of StyledStatus.
}}
`;
const StyledWarning = styled(Warning).attrs({