Skip to content

Commit

Permalink
fix(compass-connections-navigation): fix for action button not being …
Browse files Browse the repository at this point in the history
…tabbable when tabbing through navigation tree (#5923)
  • Loading branch information
himanshusinghs authored Jun 17, 2024
1 parent e3473fa commit 5f8b75b
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type NavigationBaseItemProps = {

canExpand: boolean;
isExpanded: boolean;
isFocused: boolean;
onExpand: (toggle: boolean) => void;

actionProps: {
Expand Down Expand Up @@ -58,6 +59,7 @@ export const NavigationBaseItem = ({
dataAttributes,
canExpand,
isExpanded,
isFocused,
onExpand,
}: NavigationBaseItemProps) => {
const [hoverProps, isHovered] = useHoverState();
Expand Down Expand Up @@ -88,7 +90,7 @@ export const NavigationBaseItem = ({
</ItemLabel>
</ItemButtonWrapper>
<ItemActionControls<Actions>
isVisible={isActive || isHovered}
isVisible={isActive || isHovered || isFocused}
data-testid="sidebar-navigation-item-actions"
iconSize="small"
{...actionProps}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,39 @@ describe('ConnectionsNavigationTree', function () {
});
});

it('should render the action items for the tabbed navigation item', async function () {
await renderConnectionsNavigationTree({
expanded: {},
activeWorkspace: null,
});

// Tab to the first element
userEvent.tab();
await waitFor(() => {
// Virtual list will be the one to grab the focus first, but will
// immediately forward it to the element and mocking raf here breaks
// virtual list implementatin, waitFor is to accomodate for that
expect(document.querySelector('[data-id="connection_ready"]')).to.eq(
document.activeElement
);
return true;
});
let tabbedItem = screen.getByTestId('connection_ready');
expect(within(tabbedItem).getByLabelText('Show actions')).to.be.visible;

// Go down to the second element
userEvent.keyboard('{arrowdown}');
await waitFor(() => {
expect(document.querySelector('[data-id="connection_initial"]')).to.eq(
document.activeElement
);
return true;
});

tabbedItem = screen.getByTestId('connection_initial');
expect(within(tabbedItem).getByLabelText('Show actions')).to.be.visible;
});

describe('when connection is writable', function () {
it('should show all connection actions', async function () {
await renderConnectionsNavigationTree();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,18 @@ const ConnectionsNavigationTree: React.FunctionComponent<
onDefaultAction={onDefaultAction}
onExpandedChange={onItemExpand}
getItemKey={(item) => item.id}
renderItem={({ item }) => (
<NavigationItem
item={item}
activeItemId={activeItemId}
getItemActions={getItemActions}
onItemExpand={onItemExpand}
onItemAction={onItemAction}
/>
)}
renderItem={({ item, isActive, isFocused }) => {
return (
<NavigationItem
item={item}
isActive={isActive}
isFocused={isFocused}
getItemActions={getItemActions}
onItemExpand={onItemExpand}
onItemAction={onItemAction}
/>
);
}}
/>
)}
</AutoSizer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { ConnectionStatus } from '@mongodb-js/compass-connections/provider';

type NavigationItemProps = {
item: SidebarTreeItem;
activeItemId?: string;
isActive: boolean;
isFocused: boolean;
getItemActions: (item: SidebarTreeItem) => NavigationItemActions;
onItemAction: (
item: SidebarActionableItem,
Expand All @@ -23,7 +24,8 @@ type NavigationItemProps = {

export function NavigationItem({
item,
activeItemId,
isActive,
isFocused,
onItemAction,
onItemExpand,
getItemActions,
Expand Down Expand Up @@ -119,7 +121,8 @@ export function NavigationItem({
<PlaceholderItem level={item.level} />
) : (
<NavigationBaseItem
isActive={item.id === activeItemId}
isActive={isActive}
isFocused={isFocused}
isExpanded={!!item.isExpanded}
icon={itemIcon}
name={item.name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export function useVirtualNavigationTree<T extends HTMLElement = HTMLElement>({
activeItemId?: string;
onExpandedChange(item: VirtualTreeItem, isExpanded: boolean): void;
onFocusMove?: (item: VirtualTreeItem) => void;
}): [React.HTMLProps<T>, string | undefined] {
}): [React.HTMLProps<T>, string | undefined, boolean] {
const rootRef = useRef<T | null>(null);
const activeId = activeItemId || findFirstItem(items)?.id;
const [currentTabbable, setCurrentTabbable] = useState(activeId);
Expand Down Expand Up @@ -303,5 +303,7 @@ export function useVirtualNavigationTree<T extends HTMLElement = HTMLElement>({
...focusProps,
};

return [rootProps, currentTabbable];
const isTreeItemFocused = focusState === FocusState.FocusWithinVisible;

return [rootProps, currentTabbable, isTreeItemFocused];
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ function useDefaultAction<T extends VirtualTreeItem>(
}

type NotPlaceholderTreeItem<T> = T extends { type: 'placeholder' } ? never : T;
type RenderItem<T> = (props: { index: number; item: T }) => React.ReactNode;
type RenderItem<T> = (props: {
index: number;
isActive: boolean;
isFocused: boolean;
item: T;
}) => React.ReactNode;
export type OnDefaultAction<T> = (
item: T,
evt: React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>
Expand Down Expand Up @@ -113,25 +118,33 @@ export function VirtualTree<T extends VirtualItem>({
},
[items]
);
const [rootProps, currentTabbable] = useVirtualNavigationTree<HTMLDivElement>(
{
const [rootProps, currentTabbable, isTreeItemFocused] =
useVirtualNavigationTree<HTMLDivElement>({
items,
activeItemId,
onExpandedChange,
onFocusMove,
}
);
});

const id = useId();

const itemData = useMemo(() => {
return {
items,
currentTabbable,
isTreeItemFocused,
activeItemId,
renderItem,
onDefaultAction,
};
}, [items, renderItem, currentTabbable, onDefaultAction]);
}, [
items,
renderItem,
currentTabbable,
onDefaultAction,
activeItemId,
isTreeItemFocused,
]);

const getItemKey = useCallback(
(index: number, data: VirtualItemData<T>) => {
Expand Down Expand Up @@ -169,7 +182,9 @@ export function VirtualTree<T extends VirtualItem>({

type VirtualItemData<T extends VirtualItem> = {
items: T[];
isTreeItemFocused: boolean;
currentTabbable?: string;
activeItemId?: string;
renderItem: RenderItem<T>;
onDefaultAction: OnDefaultAction<NotPlaceholderTreeItem<T>>;
};
Expand All @@ -178,13 +193,28 @@ function TreeItem<T extends VirtualItem>({
data,
style,
}: ListChildComponentProps<VirtualItemData<T>>) {
const { renderItem, items } = data;
const { renderItem, items, activeItemId } = data;
const item = useMemo(() => items[index], [items, index]);
const focusRingProps = useFocusRing();

const component = useMemo(() => {
return renderItem({ index, item });
}, [renderItem, index, item]);
return renderItem({
index,
item,
isActive: !isPlaceholderItem(item) && item.id === activeItemId,
isFocused:
data.isTreeItemFocused &&
!isPlaceholderItem(item) &&
item.id === data.currentTabbable,
});
}, [
renderItem,
index,
item,
activeItemId,
data.currentTabbable,
data.isTreeItemFocused,
]);

const actionProps = useDefaultAction(
item as NotPlaceholderTreeItem<T>,
Expand Down

0 comments on commit 5f8b75b

Please sign in to comment.