Skip to content

Commit

Permalink
Connect: Show all recent connections in connection list (#40197)
Browse files Browse the repository at this point in the history
* Show all items of FilterableList

* Scroll to active item when navigating with arrows

* Open connections in story only if list is closed
  • Loading branch information
ravicious authored Apr 5, 2024
1 parent b50eecb commit e02ed57
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import { useRef, useEffect } from 'react';
import { Flex, Label, Text } from 'design';

import styled from 'styled-components';
Expand All @@ -33,15 +33,21 @@ interface ClusterItemProps {
}

export function ClusterItem(props: ClusterItemProps) {
const { isActive } = useKeyboardArrowsNavigation({
const { isActive, scrollIntoViewIfActive } = useKeyboardArrowsNavigation({
index: props.index,
onRun: props.onSelect,
});
const ref = useRef<HTMLElement>();

const clusterName = props.item.name;

useEffect(() => {
scrollIntoViewIfActive(ref.current);
}, [scrollIntoViewIfActive]);

return (
<StyledListItem
ref={ref}
onClick={props.onSelect}
isActive={isActive}
isSelected={props.isSelected}
Expand Down
121 changes: 88 additions & 33 deletions web/packages/teleterm/src/ui/TopBar/Connections/Connections.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import { useLayoutEffect } from 'react';
import { Flex, Text } from 'design';

import AppContextProvider from 'teleterm/ui/appContextProvider';
import { MockAppContext } from 'teleterm/ui/fixtures/mocks';
Expand Down Expand Up @@ -62,6 +63,48 @@ export function JustVnet() {
);
}

export function WithScroll() {
const appContext = new MockAppContext();
prepareAppContext(appContext);
appContext.connectionTracker.getConnections = () => [
{
connected: false,
kind: 'connection.server' as const,
title: 'last-item',
id: 'last-item',
serverUri: '/clusters/foo/servers/last-item',
login: 'item',
clusterName: 'teleport.example.sh',
},
...Array(10)
.fill(undefined)
.flatMap((_, index) => makeConnections(index)),
];

return (
<Flex
flexDirection="row"
justifyContent="space-between"
gap={3}
minWidth="500px"
maxWidth="600px"
>
<AppContextProvider value={appContext}>
<VnetContextProvider>
<Connections />
</VnetContextProvider>
</AppContextProvider>
<Text
css={`
max-width: 20ch;
`}
>
Manipulate window height to simulate how the list behaves in Connect.
</Text>
</Flex>
);
}

export function WithoutVnet() {
const appContext = new MockAppContext({ platform: 'win32' });
prepareAppContext(appContext);
Expand All @@ -87,40 +130,44 @@ export function EmptyWithoutVnet() {
);
}

const makeConnections = (index = 0) => {
const suffix = index === 0 ? '' : `-${index}`;

return [
{
connected: true,
kind: 'connection.server' as const,
title: 'ansible' + suffix,
id: 'e9c4fbc2' + suffix,
serverUri: '/clusters/foo/servers/ansible' + suffix,
login: 'casey',
clusterName: 'teleport.example.sh',
},
{
connected: true,
kind: 'connection.gateway' as const,
title: 'postgres' + suffix,
targetName: 'postgres',
id: '68b6a281' + suffix,
targetUri: '/clusters/foo/dbs/brock' + suffix,
port: '22',
gatewayUri: '/gateways/empty',
clusterName: 'teleport.example.sh',
},
{
connected: false,
kind: 'connection.server' as const,
title: 'ansible-staging' + suffix,
id: '949651ed' + suffix,
serverUri: '/clusters/foo/servers/ansible-staging' + suffix,
login: 'casey',
clusterName: 'teleport.example.sh',
},
];
};

const prepareAppContext = (appContext: MockAppContext) => {
appContext.connectionTracker.getConnections = () => {
return [
{
connected: true,
kind: 'connection.server',
title: 'ansible',
id: 'e9c4fbc2',
serverUri: '/clusters/foo/servers/ansible',
login: 'casey',
clusterName: 'teleport.example.sh',
},
{
connected: true,
kind: 'connection.gateway',
title: 'postgres',
targetName: 'postgres',
id: '68b6a281',
targetUri: '/clusters/foo/dbs/brock',
port: '22',
gatewayUri: '/gateways/empty',
clusterName: 'teleport.example.sh',
},
{
connected: false,
kind: 'connection.server',
title: 'ansible-staging',
id: '949651ed',
serverUri: '/clusters/foo/servers/ansible-staging',
login: 'casey',
clusterName: 'teleport.example.sh',
},
];
};
appContext.connectionTracker.getConnections = () => makeConnections();
appContext.connectionTracker.activateItem = async () => {};
appContext.connectionTracker.disconnectItem = async () => {};
appContext.connectionTracker.removeItem = async () => {};
Expand All @@ -130,6 +177,14 @@ const prepareAppContext = (appContext: MockAppContext) => {

const useOpenConnections = () => {
useLayoutEffect(() => {
const areConnectionsOpen = !!document.querySelector(
'input[role=searchbox]'
);

if (areConnectionsOpen) {
return;
}

const button = document.querySelector(
'button[title~="connections"i]'
) as HTMLButtonElement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import { useEffect, useRef } from 'react';
import { ButtonIcon, Flex, Text } from 'design';
import { Trash, Unlink } from 'design/Icon';

Expand All @@ -41,7 +41,7 @@ interface ConnectionItemProps {

export function ConnectionItem(props: ConnectionItemProps) {
const offline = !props.item.connected;
const { isActive } = useKeyboardArrowsNavigation({
const { isActive, scrollIntoViewIfActive } = useKeyboardArrowsNavigation({
index: props.index,
onRun: props.onActivate,
});
Expand All @@ -60,11 +60,17 @@ export function ConnectionItem(props: ConnectionItemProps) {
};

const actionIcon = offline ? actionIcons.remove : actionIcons.disconnect;
const ref = useRef<HTMLElement>();

useEffect(() => {
scrollIntoViewIfActive(ref.current);
}, [scrollIntoViewIfActive]);

return (
<ListItem
onClick={props.onActivate}
isActive={isActive}
ref={ref}
css={`
padding: 6px 8px;
height: unset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function Node({ item }: { item: TestItem }) {
return <li>{item.title}</li>;
}

test('render first 10 items by default', () => {
test('render all items by default', () => {
render(
<FilterableList<TestItem>
items={mockedItems}
Expand All @@ -44,7 +44,7 @@ test('render first 10 items by default', () => {
);
const items = screen.getAllByRole('listitem');

expect(items).toHaveLength(10);
expect(items).toHaveLength(30);
items.forEach((item, index) => {
expect(item).toHaveTextContent(mockedItems[index].title);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,14 @@ interface FilterableListProps<T> {
onFilterChange?(filter: string): void;
}

const maxItemsToShow = 10;

export function FilterableList<T>(
props: React.PropsWithChildren<FilterableListProps<T>>
) {
const { items } = props;
const [searchValue, setSearchValue] = useState<string>();

const filteredItems = useMemo(
() =>
filterItems(searchValue, items, props.filterBy).slice(0, maxItemsToShow),
() => filterItems(searchValue, items, props.filterBy),
[items, searchValue]
);

Expand Down Expand Up @@ -84,7 +81,7 @@ const UnorderedList = styled.ul`
`;

const StyledInput = styled(Input)`
background: inherit;
background-color: inherit;
border-radius: 51px;
margin-bottom: 8px;
font-size: 14px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { useContext, useEffect } from 'react';
import { useCallback, useContext, useEffect } from 'react';

import {
KeyboardArrowsNavigationContext,
Expand Down Expand Up @@ -48,8 +48,35 @@ export function useKeyboardArrowsNavigation({
return () => navigationContext.removeItem(index);
}, [index, onRun, navigationContext.addItem, navigationContext.removeItem]);

const isActive = index === navigationContext.activeIndex;

const scrollIntoViewIfActive = useCallback(
(el: HTMLElement | undefined) => {
if (!isActive || !el) {
return;
}

// By default, scrollIntoView uses 'start'. This is a problem in two cases:
//
// 1. When scrolling from the last to the first element, the top of the scrollable area gets
// aligned to the top of the first active element, not to the top of the parent container.
// 2. When scrolling from any other element to the next one, the scrollable area gets aligned
// to the top of the active element, meaning that the previous element immediately disappears.
//
// 'center' fixes both problems, while being closer than 'nearest' to how the browser adjusts
// the scrollable area when tabbing through focusable elements. It ensures that you see what's
// after and before the active element.
//
// Compared to 'nearest', it also makes sure that the scrollable area is aligned to its bottom
// when scrolling to the last element – 'nearest' aligns it to the bottom of the active item.
el.scrollIntoView({ block: 'center' });
},
[isActive]
);

return {
isActive: index === navigationContext.activeIndex,
isActive,
scrollIntoViewIfActive,
};
}

Expand Down

0 comments on commit e02ed57

Please sign in to comment.