Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function ActionPicker(props: { input: ReactElement }) {
}
}

let ExtraComponent = null;
let ExtraTopComponent = null;
// The order of attempts is important. Filter actions should be displayed before resource actions.
const attempts = [filterActionsAttempt, resourceActionsAttempt];
const attemptsHaveFinishedWithoutActions = attempts.every(
Expand All @@ -137,13 +137,13 @@ export function ActionPicker(props: { input: ReactElement }) {
filterActionsAttempt.data.length === 0;

if (inputValue && attemptsHaveFinishedWithoutActions) {
ExtraComponent = (
ExtraTopComponent = (
<NoResultsItem clusters={clustersService.getRootClusters()} />
);
}

if (!inputValue && noRemainingFilters) {
ExtraComponent = <TypeToSearchItem />;
ExtraTopComponent = <TypeToSearchItem />;
}

return (
Expand Down Expand Up @@ -171,7 +171,7 @@ export function ActionPicker(props: { input: ReactElement }) {
),
};
}}
ExtraComponent={ExtraComponent}
ExtraTopComponent={ExtraTopComponent}
/>
</PickerContainer>
);
Expand Down
28 changes: 17 additions & 11 deletions web/packages/teleterm/src/ui/Search/pickers/ResultList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import React, {
useRef,
useState,
} from 'react';
import styled from 'styled-components';
import styled, { css } from 'styled-components';

import { Attempt } from 'shared/hooks/useAsync';

Expand All @@ -35,16 +35,16 @@ type ResultListProps<T> = {
*/
attempts: Attempt<T[]>[];
/**
* ExtraComponent is the element that is rendered above the items.
* ExtraTopComponent is the element that is rendered above the items.
*/
ExtraComponent?: ReactElement;
ExtraTopComponent?: ReactElement;
onPick(item: T): void;
onBack(): void;
render(item: T): { Component: ReactElement; key: string };
};

export function ResultList<T>(props: ResultListProps<T>) {
const { attempts, ExtraComponent, onPick, onBack } = props;
const { attempts, ExtraTopComponent, onPick, onBack } = props;
const activeItemRef = useRef<HTMLDivElement>();
const [activeItemIndex, setActiveItemIndex] = useState(0);

Expand Down Expand Up @@ -110,7 +110,7 @@ export function ResultList<T>(props: ResultListProps<T>) {
)}
</Separator>
<Overflow role="menu">
{ExtraComponent}
{ExtraTopComponent}
{items.map((r, index) => {
const isActive = index === activeItemIndex;
const { Component, key } = props.render(r);
Expand All @@ -119,7 +119,7 @@ export function ResultList<T>(props: ResultListProps<T>) {
<InteractiveItem
ref={isActive ? activeItemRef : null}
role="menuitem"
$active={isActive}
active={isActive}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's the convention behind adding a dollar sign to props?

https://styled-components.com/docs/api#transient-props

(it does not have any effect here, so I removed it)

key={key}
onClick={() => props.onPick(r)}
>
Expand All @@ -145,18 +145,24 @@ export const NonInteractiveItem = styled.div`

padding: ${props => props.theme.space[2]}px;
color: ${props => props.theme.colors.text.contrast};
background: ${props =>
props.$active
? props.theme.colors.levels.elevated
: props.theme.colors.levels.surface};
background: ${props => props.theme.colors.levels.surface};
`;

const InteractiveItem = styled(NonInteractiveItem)`
cursor: pointer;

&:hover,
&:focus {
cursor: pointer;
background: ${props => props.theme.colors.levels.elevated};
}

${props => {
if (props.active) {
return css`
background: ${props => props.theme.colors.levels.elevated};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the CSS that ends up being generated. It shouldn't cause problems here but I'd be wary of using this approach in general.

generated CSS

`;
}
}}
`;

function getNext(selectedIndex = 0, max = 0) {
Expand Down
69 changes: 68 additions & 1 deletion web/packages/teleterm/src/ui/Search/useSearch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import {
makeKube,
makeLabelsList,
} from './searchResultTestHelpers';
import { rankResults, useResourceSearch } from './useSearch';
import { rankResults, useFilterSearch, useResourceSearch } from './useSearch';

import type * as tsh from 'teleterm/services/tshd/types';

describe('rankResults', () => {
it('uses the displayed resource name as the tie breaker if the scores are equal', () => {
Expand Down Expand Up @@ -156,3 +158,68 @@ describe('useResourceSearch', () => {
expect(searchResult.results).toEqual(servers);
});
});

describe('useFiltersSearch', () => {
it('does not return cluster filters if there is only one cluster', () => {
const appContext = new MockAppContext();
appContext.clustersService.setState(draftState => {
draftState.clusters.set('/clusters/teleport-local', {
connected: true,
authClusterId: '73c4746b-d956-4f16-9848-4e3469f70762',
leaf: false,
name: 'teleport-local',
proxyHost: 'localhost:3080',
uri: '/clusters/teleport-local',
});
});

const { result } = renderHook(() => useFilterSearch(), {
wrapper: ({ children }) => (
<MockAppContextProvider appContext={appContext}>
{children}
</MockAppContextProvider>
),
});
const clusterFilters = result
.current('', [])
.filter(f => f.kind === 'cluster-filter');
expect(clusterFilters).toHaveLength(0);
});

it('returns one cluster filter if the search term matches it', () => {
const appContext = new MockAppContext();
const clusterA: tsh.Cluster = {
connected: true,
authClusterId: '73c4746b-d956-4f16-9848-4e3469f70762',
leaf: false,
name: 'teleport-a',
proxyHost: 'localhost:3080',
uri: '/clusters/teleport-a',
};
const clusterB: tsh.Cluster = {
connected: true,
authClusterId: '73c4746b-d956-4f16-1848-4e3469f70762',
leaf: false,
name: 'teleport-b',
proxyHost: 'localhost:3080',
uri: '/clusters/teleport-b',
};
appContext.clustersService.setState(draftState => {
draftState.clusters.set(clusterA.uri, clusterA);
draftState.clusters.set(clusterB.uri, clusterB);
});

const { result } = renderHook(() => useFilterSearch(), {
wrapper: ({ children }) => (
<MockAppContextProvider appContext={appContext}>
{children}
</MockAppContextProvider>
),
});
const clusterFilters = result
.current('teleport-a', [])
.filter(f => f.kind === 'cluster-filter');
expect(clusterFilters).toHaveLength(1);
expect(clusterFilters[0].resource).toEqual(clusterA);
});
});
8 changes: 4 additions & 4 deletions web/packages/teleterm/src/ui/Search/useSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,17 @@ export function useFilterSearch() {
(search: string, restrictions: SearchFilter[]): FilterSearchResult[] => {
const getClusters = () => {
let clusters = clustersService.getClusters();
// Cluster filter should not be visible if there is only one cluster
if (clusters.length === 1) {
return [];
}
if (search) {
clusters = clusters.filter(cluster =>
cluster.name
.toLocaleLowerCase()
.includes(search.toLocaleLowerCase())
);
}
// Cluster filter should not be visible if there is only one cluster
if (clusters.length === 1) {
return [];
}
return clusters.map(cluster => {
let score = getLengthScore(search, cluster.name);
if (
Expand Down