Skip to content

Show errors if fetching ParameterPicker suggestions fails #25703

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
ravicious merged 4 commits into from
May 8, 2023
Merged

Show errors if fetching ParameterPicker suggestions fails #25703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f23453
ResultList: Reset active item on pick
ravicious May 5, 2023
8b75bf4
Extract inner item component with icon to ResultList
ravicious May 5, 2023
0f140ce
Remove hardcoded icon color of ResourceSearchErrorsItem
ravicious May 5, 2023
78182ba
Handle errors when getting suggestions for ParameterPicker
ravicious May 5, 2023
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
51 changes: 15 additions & 36 deletions web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import { CrossClusterResourceSearchResult } from '../useSearch';

import { useActionAttempts } from './useActionAttempts';
import { getParameterPicker } from './pickers';
import { ResultList, NonInteractiveItem } from './ResultList';
import { ResultList, NonInteractiveItem, IconAndContent } from './ResultList';
import { PickerContainer } from './PickerContainer';

export function ActionPicker(props: { input: ReactElement }) {
Expand Down Expand Up @@ -374,30 +374,9 @@ type SearchResultItem<T> = {
getOptionalClusterName: (uri: uri.ResourceUri) => string;
};

function Item(
props: React.PropsWithChildren<{
Icon: React.ComponentType<{
color: string;
fontSize: string;
lineHeight: string;
}>;
iconColor: string;
}>
) {
return (
<Flex alignItems="flex-start" gap={2}>
{/* lineHeight of the icon needs to match the line height of the first row of props.children */}
<props.Icon color={props.iconColor} fontSize="20px" lineHeight="24px" />
<Flex flexDirection="column" gap={1} minWidth={0} flex="1">
{props.children}
</Flex>
</Flex>
);
}

function ClusterFilterItem(props: SearchResultItem<SearchResultCluster>) {
return (
<Item Icon={icons.Lan} iconColor="text.slightlyMuted">
<IconAndContent Icon={icons.Lan} iconColor="text.slightlyMuted">
<Text typography="body1">
Search only in{' '}
<strong>
Expand All @@ -407,7 +386,7 @@ function ClusterFilterItem(props: SearchResultItem<SearchResultCluster>) {
/>
</strong>
</Text>
</Item>
</IconAndContent>
);
}

Expand All @@ -428,7 +407,7 @@ function ResourceTypeFilterItem(
props: SearchResultItem<SearchResultResourceType>
) {
return (
<Item
<IconAndContent
Icon={resourceIcons[props.searchResult.resource]}
iconColor="text.slightlyMuted"
>
Expand All @@ -441,7 +420,7 @@ function ResourceTypeFilterItem(
/>
</strong>
</Text>
</Item>
</IconAndContent>
);
}

Expand All @@ -453,7 +432,7 @@ export function ServerItem(props: SearchResultItem<SearchResultServer>) {
);

return (
<Item Icon={icons.Server} iconColor="brand">
<IconAndContent Icon={icons.Server} iconColor="brand">
<Flex
justifyContent="space-between"
alignItems="center"
Expand Down Expand Up @@ -493,7 +472,7 @@ export function ServerItem(props: SearchResultItem<SearchResultServer>) {
)}
</ResourceFields>
</Labels>
</Item>
</IconAndContent>
);
}

Expand Down Expand Up @@ -527,7 +506,7 @@ export function DatabaseItem(props: SearchResultItem<SearchResultDatabase>) {
);

return (
<Item Icon={icons.Database} iconColor="brand">
<IconAndContent Icon={icons.Database} iconColor="brand">
<Flex
justifyContent="space-between"
alignItems="center"
Expand Down Expand Up @@ -558,15 +537,15 @@ export function DatabaseItem(props: SearchResultItem<SearchResultDatabase>) {
) : (
<Labels searchResult={searchResult}>{$resourceFields}</Labels>
)}
</Item>
</IconAndContent>
);
}

export function KubeItem(props: SearchResultItem<SearchResultKube>) {
const { searchResult } = props;

return (
<Item Icon={icons.Kubernetes} iconColor="brand">
<IconAndContent Icon={icons.Kubernetes} iconColor="brand">
<Flex
justifyContent="space-between"
alignItems="center"
Expand All @@ -587,7 +566,7 @@ export function KubeItem(props: SearchResultItem<SearchResultKube>) {
</Flex>

<Labels searchResult={searchResult} />
</Item>
</IconAndContent>
);
}

Expand All @@ -613,10 +592,10 @@ export function NoResultsItem(props: {

return (
<NonInteractiveItem>
<Item Icon={icons.Info} iconColor="text.slightlyMuted">
<IconAndContent Icon={icons.Info} iconColor="text.slightlyMuted">
<Text typography="body1">No matching results found.</Text>
{expiredCertsCopy && <Text typography="body2">{expiredCertsCopy}</Text>}
</Item>
</IconAndContent>
</NonInteractiveItem>
);
}
Expand Down Expand Up @@ -660,7 +639,7 @@ export function ResourceSearchErrorsItem(props: {

return (
<NonInteractiveItem>
<Item Icon={icons.Warning} iconColor="#f3af3d">
<IconAndContent Icon={icons.Warning} iconColor="warning.main">
<Text typography="body1">
Some of the search results are incomplete.
</Text>
Expand All @@ -687,7 +666,7 @@ export function ResourceSearchErrorsItem(props: {
Show details
</ButtonBorder>
</Flex>
</Item>
</IconAndContent>
</NonInteractiveItem>
);
}
Expand Down
29 changes: 25 additions & 4 deletions web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ import {
mapAttempt,
useAsync,
} from 'shared/hooks/useAsync';
import { Text } from 'design';
import * as icons from 'design/Icon';

import { useSearchContext } from '../SearchContext';
import { ParametrizedAction } from '../actions';

import { ResultList } from './ResultList';
import { IconAndContent, NonInteractiveItem, ResultList } from './ResultList';
import { actionPicker } from './pickers';
import { PickerContainer } from './PickerContainer';

Expand All @@ -42,14 +44,21 @@ export function ParameterPicker(props: ParameterPickerProps) {
resetInput,
addWindowEventListener,
} = useSearchContext();
const [suggestionsAttempt, fetch] = useAsync(
const [suggestionsAttempt, getSuggestions] = useAsync(
props.action.parameter.getSuggestions
);
const inputSuggestionAttempt = makeSuccessAttempt(inputValue && [inputValue]);
const $suggestionsError =
suggestionsAttempt.status === 'error' ? (
<SuggestionsError statusText={suggestionsAttempt.statusText} />
) : null;

useEffect(() => {
fetch();
}, [props.action]);
getSuggestions();
// We want to get suggestions only once on mount.
// useAsync already handles cleanup and calling the hook twice.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const attempt = mapAttempt(suggestionsAttempt, suggestions =>
suggestions.filter(
Expand Down Expand Up @@ -80,6 +89,7 @@ export function ParameterPicker(props: ParameterPickerProps) {
{props.input}
<ResultList<string>
attempts={[inputSuggestionAttempt, attempt]}
ExtraTopComponent={$suggestionsError}
onPick={onPick}
onBack={onBack}
addWindowEventListener={addWindowEventListener}
Expand All @@ -93,3 +103,14 @@ export function ParameterPicker(props: ParameterPickerProps) {
</PickerContainer>
);
}

export const SuggestionsError = ({ statusText }: { statusText: string }) => (
<NonInteractiveItem>
<IconAndContent Icon={icons.Warning} iconColor="warning.main">
<Text typography="body1">
Could not fetch suggestions. Type in the desired value to continue.
</Text>
<Text typography="body2">{statusText}</Text>
</IconAndContent>
</NonInteractiveItem>
);
45 changes: 42 additions & 3 deletions web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import React, {
useMemo,
useRef,
useState,
useCallback,
} from 'react';
import { Flex } from 'design';
import styled, { css } from 'styled-components';
import { Attempt } from 'shared/hooks/useAsync';

Expand Down Expand Up @@ -55,6 +57,13 @@ export function ResultList<T>(props: ResultListProps<T>) {
} = props;
const activeItemRef = useRef<HTMLDivElement>();
const [activeItemIndex, setActiveItemIndex] = useState(0);
const pickAndResetActiveItem = useCallback(
(item: T) => {
onPick(item);
setActiveItemIndex(0);
},
[onPick]
);

const items = useMemo(() => {
return attempts.map(a => a.data || []).flat();
Expand Down Expand Up @@ -83,7 +92,7 @@ export function ResultList<T>(props: ResultListProps<T>) {

const item = items[activeItemIndex];
if (item) {
onPick(item);
pickAndResetActiveItem(item);
}
break;
}
Expand All @@ -110,7 +119,13 @@ export function ResultList<T>(props: ResultListProps<T>) {
capture: true,
});
return cleanup;
}, [items, onPick, onBack, activeItemIndex, addWindowEventListener]);
}, [
items,
pickAndResetActiveItem,
onBack,
activeItemIndex,
addWindowEventListener,
]);

return (
<>
Expand All @@ -131,7 +146,7 @@ export function ResultList<T>(props: ResultListProps<T>) {
role="menuitem"
active={isActive}
key={key}
onClick={() => props.onPick(r)}
onClick={() => pickAndResetActiveItem(r)}
>
{Component}
</InteractiveItem>
Expand Down Expand Up @@ -173,6 +188,30 @@ const InteractiveItem = styled(NonInteractiveItem)`
}}
`;

/**
* IconAndContent is supposed to be used within InteractiveItem & NonInteractiveItem.
*/
export function IconAndContent(
Copy link
Copy Markdown
Member Author

@ravicious ravicious May 5, 2023

Choose a reason for hiding this comment

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

I didn't have better idea for a name. 🤕

Copy link
Copy Markdown
Contributor

@avatus avatus May 5, 2023

Choose a reason for hiding this comment

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

I tried to come up with a suggestion and I couldn't either. Sounds like its good enough to me

props: React.PropsWithChildren<{
Icon: React.ComponentType<{
color: string;
fontSize: string;
lineHeight: string;
}>;
iconColor: string;
}>
) {
return (
<Flex alignItems="flex-start" gap={2}>
{/* lineHeight of the icon needs to match the line height of the first row of props.children */}
<props.Icon color={props.iconColor} fontSize="20px" lineHeight="24px" />
<Flex flexDirection="column" gap={1} minWidth={0} flex="1">
{props.children}
</Flex>
</Flex>
);
}

function getNext(selectedIndex = 0, max = 0) {
let index = selectedIndex % max;
if (index < 0) {
Expand Down
14 changes: 10 additions & 4 deletions .../ui/Search/pickers/ActionPicker.story.tsx → ...m/src/ui/Search/pickers/results.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,20 @@ import {
ResourceSearchErrorsItem,
TypeToSearchItem,
} from './ActionPicker';
import { SuggestionsError } from './ParameterPicker';
import { ResultList } from './ResultList';

import type * as uri from 'teleterm/ui/uri';

export default {
title: 'Teleterm/Search/ActionPicker',
title: 'Teleterm/Search',
};

const clusterUri: uri.ClusterUri = '/clusters/teleport-local';
const longClusterUri: uri.ClusterUri =
'/clusters/teleport-very-long-cluster-name-with-uuid-2f96e498-88ec-442f-a25b-569fa915041c';

export const Items = (props: { maxWidth: string }) => {
export const Results = (props: { maxWidth: string }) => {
const { maxWidth = '600px' } = props;

return (
Expand Down Expand Up @@ -92,8 +93,8 @@ export const Items = (props: { maxWidth: string }) => {
);
};

export const ItemsNarrow = () => {
return <Items maxWidth="300px" />;
export const ResultsNarrow = () => {
return <Results maxWidth="300px" />;
};

const SearchResultItems = () => {
Expand Down Expand Up @@ -397,6 +398,11 @@ const AuxiliaryItems = () => (
),
]}
/>
<SuggestionsError
Copy link
Copy Markdown
Member Author

@ravicious ravicious May 5, 2023

Choose a reason for hiding this comment

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

I moved the story from ActionPicker.story.tsx to just results.story.tsx. We could have separate stories for ActionPicker and ParameterPicker but I'm not sure if it makes sense at the moment, we just have a single item from ParameterPicker here. I like being able to see them all at once.

Copy link
Copy Markdown
Contributor

@gzdunek gzdunek May 5, 2023

Choose a reason for hiding this comment

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

It's fine to me.

statusText={
'2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host'
}
/>
<TypeToSearchItem hasNoRemainingFilterActions={false} />
<TypeToSearchItem hasNoRemainingFilterActions={true} />
</>
Expand Down