Skip to content
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.

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

Feature/combobox pageup and pagedown #3158

Merged
merged 19 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
abe85e6
Add support for PageUp and PageDown in Combobox.FilteredOptions
it-vegard Aug 7, 2024
04ec307
Remove console.log statements
it-vegard Aug 16, 2024
ab49f11
Add test for PageDown/PageUp
it-vegard Aug 16, 2024
d7d89d4
Open FilteredOptions when pressing PageDown
it-vegard Aug 16, 2024
d8badb8
Prevent scrolling from moving the whole page
it-vegard Sep 18, 2024
7252e8d
Consistently scroll to the active element in useVirtualFocus by using…
it-vegard Sep 18, 2024
0b2d872
Updated test to fit new options order
it-vegard Sep 18, 2024
f545135
Prevent "Home" and "End" in Combobox from scrolling as well
it-vegard Sep 18, 2024
bbaa81f
Add changeset
it-vegard Sep 18, 2024
4787f13
Update @navikt/core/react/src/form/combobox/FilteredOptions/useVirtua…
it-vegard Sep 20, 2024
fa61611
useEffect seems to work the same way, and works better for SSR
it-vegard Sep 20, 2024
bcfd178
Removed code that did nothing
it-vegard Sep 20, 2024
a3b8350
Added the same check we have for PageDown, etc. Unsure if it is necce…
it-vegard Sep 20, 2024
5cadfaf
Merge branch 'main' into feature/combobox-pageup-and-pagedown
it-vegard Sep 20, 2024
4600f02
Merge branch 'main' into feature/combobox-pageup-and-pagedown
it-vegard Oct 9, 2024
f6184c6
Change behaviour for "Home" to move focus to the first element instea…
it-vegard Oct 9, 2024
6184d07
Merge branch 'main' into feature/combobox-pageup-and-pagedown
it-vegard Oct 9, 2024
215f9e7
Once we changed virtualFocus.moveFocusToTop to move focus to first el…
it-vegard Oct 10, 2024
378b340
Update .changeset/tasty-cats-hear.md
it-vegard Oct 10, 2024
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
6 changes: 6 additions & 0 deletions .changeset/tasty-cats-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@navikt/ds-react": patch
"@navikt/ds-css": patch
---

Combobox: Support PageUp/PageDown in dropdown list.
1 change: 1 addition & 0 deletions @navikt/core/css/form/combobox.css
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@
border-radius: var(--a-border-radius-medium);
background-color: var(--ac-combobox-list-bg, var(--a-surface-default));
color: var(--ac-combobox-list-text, var(--a-text-default));
overscroll-behavior: contain;
}

.navds-combobox__list--closed {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ const FilteredOptionsProvider = ({
if (disabled || readOnly) {
return;
}
virtualFocus.moveFocusToTop();
virtualFocus.resetFocus();
if (newState ?? !isInternalListOpen) {
setHideCaret(!!maxSelected?.isLimitReached);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from "react";
import { useEffect, useState } from "react";

export type VirtualFocusType = {
activeElement: HTMLElement | undefined;
Expand All @@ -10,6 +10,9 @@ export type VirtualFocusType = {
moveFocusToElement: (id: string) => void;
moveFocusToTop: () => void;
moveFocusToBottom: () => void;
moveFocusUpBy: (numberOfElements: number) => void;
moveFocusDownBy: (numberOfElements: number) => void;
resetFocus: () => void;
};

const useVirtualFocus = (
Expand Down Expand Up @@ -40,11 +43,6 @@ const useVirtualFocus = (
: false;
};

const _moveFocusAndScrollTo = (_element?: HTMLElement) => {
setActiveElement(_element);
_element?.scrollIntoView?.({ block: "nearest" });
};

const moveFocusUp = () => {
if (!activeElement) {
return;
Expand All @@ -55,28 +53,32 @@ const useVirtualFocus = (
if (_currentIndex === 0) {
setActiveElement(undefined);
} else {
_moveFocusAndScrollTo(elementAbove);
setActiveElement(elementAbove);
}
};

const moveFocusDown = () => {
const elementsAbleToReceiveFocus = getElementsAbleToReceiveFocus();
if (!activeElement) {
_moveFocusAndScrollTo(elementsAbleToReceiveFocus[0]);
setActiveElement(elementsAbleToReceiveFocus[0]);
return;
}
const _currentIndex = elementsAbleToReceiveFocus.indexOf(activeElement);
if (_currentIndex === elementsAbleToReceiveFocus.length - 1) {
return;
}

_moveFocusAndScrollTo(elementsAbleToReceiveFocus[_currentIndex + 1]);
setActiveElement(elementsAbleToReceiveFocus[_currentIndex + 1]);
};

const moveFocusToTop = () => _moveFocusAndScrollTo(undefined);
const resetFocus = () => setActiveElement(undefined);
const moveFocusToTop = () => {
const elementsAbleToReceiveFocus = getElementsAbleToReceiveFocus();
setActiveElement(elementsAbleToReceiveFocus[0]);
};
const moveFocusToBottom = () => {
const elementsAbleToReceiveFocus = getElementsAbleToReceiveFocus();
return _moveFocusAndScrollTo(
setActiveElement(
elementsAbleToReceiveFocus[elementsAbleToReceiveFocus.length - 1],
);
};
Expand All @@ -89,6 +91,32 @@ const useVirtualFocus = (
}
};

const moveFocusUpBy = (numberOfElements: number) => {
if (!activeElement) {
return;
}
const elementsAbleToReceiveFocus = getElementsAbleToReceiveFocus();
const currentIndex = elementsAbleToReceiveFocus.indexOf(activeElement);
const newIndex = Math.max(currentIndex - numberOfElements, 0);
setActiveElement(elementsAbleToReceiveFocus[newIndex]);
};

const moveFocusDownBy = (numberOfElements: number) => {
const elementsAbleToReceiveFocus = getElementsAbleToReceiveFocus();
const currentIndex = activeElement
? elementsAbleToReceiveFocus.indexOf(activeElement)
: -1;
const newIndex = Math.min(
currentIndex + numberOfElements,
elementsAbleToReceiveFocus.length - 1,
);
setActiveElement(elementsAbleToReceiveFocus[newIndex]);
};

useEffect(() => {
activeElement?.scrollIntoView?.({ block: "nearest" });
}, [activeElement]);

return {
activeElement,
getElementById,
Expand All @@ -99,6 +127,9 @@ const useVirtualFocus = (
moveFocusToElement,
moveFocusToTop,
moveFocusToBottom,
moveFocusUpBy,
moveFocusDownBy,
resetFocus,
};
};

Expand Down
29 changes: 19 additions & 10 deletions @navikt/core/react/src/form/combobox/Input/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,6 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
case "Accept":
onEnter(e);
break;
case "Home":
toggleIsListOpen(false);
virtualFocus.moveFocusToTop();
break;
case "End":
toggleIsListOpen(true);
virtualFocus.moveFocusToBottom();
break;
default:
break;
}
Expand Down Expand Up @@ -202,6 +194,24 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
}
virtualFocus.moveFocusUp();
}
} else if (e.key === "Home") {
e.preventDefault();
virtualFocus.moveFocusToTop();
} else if (e.key === "End") {
e.preventDefault();
if (virtualFocus.activeElement === null || !isListOpen) {
toggleIsListOpen(true);
}
virtualFocus.moveFocusToBottom();
} else if (e.key === "PageUp") {
e.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
e.preventDefault();
e.preventDefault();
if (virtualFocus.isFocusOnTheTop()) {
toggleIsListOpen(false);
}

Funket som bare det! (ref det du kommenterte om), var copy paste fra kode over 😎. Tror jeg liker at det er lik behaviour på tvers av alle pagineringsmuligheter 🤔.

virtualFocus.moveFocusUpBy(6);
} else if (e.key === "PageDown") {
e.preventDefault();
if (virtualFocus.activeElement === null || !isListOpen) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to check this for } else if (e.key === "End") to? toggleIsListOpen(true); is always called now when end is pressed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Maybe. I am unsure if there is a problem with actually doing this. One thing is ArrowDown/-Up, which the user could press-and-hold, but Home/End and PageUp/-Down shouldn't cause a performance hit even IF this causes any updates.

Another thing is an inconsistency where ArrowUp from the first element and pressing Home will both close the list, whereas PageUp will stop on the first element. Should we make this more consistent?

toggleIsListOpen(true);
}
virtualFocus.moveFocusDownBy(6);
}
},
[
Expand Down Expand Up @@ -230,10 +240,9 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
} else if (filteredOptions.length === 0) {
toggleIsListOpen(false);
}
virtualFocus.moveFocusToTop();
onChange(newValue);
},
[filteredOptions.length, virtualFocus, onChange, toggleIsListOpen],
[filteredOptions.length, onChange, toggleIsListOpen],
);

return (
Expand Down
36 changes: 36 additions & 0 deletions @navikt/core/react/src/form/combobox/__tests__/combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,40 @@ describe("Render combobox", () => {
);
});
});

describe("has keyboard navigation", () => {
test("for PageDown and PageUp", async () => {
render(<App options={options} />);

const combobox = screen.getByRole("combobox", {
name: "Hva er dine favorittfrukter?",
});

const pressKey = async (key: string) => {
await act(async () => {
await userEvent.keyboard(`{${key}}`);
});
};

const hasVirtualFocus = (option: string) =>
expect(combobox.getAttribute("aria-activedescendant")).toBe(
screen.getByRole("option", { name: option }).id,
);

await act(async () => {
await userEvent.click(combobox);
});

await pressKey("ArrowDown");
hasVirtualFocus("apple");
await pressKey("PageDown");
hasVirtualFocus("mango");
await pressKey("PageDown");
hasVirtualFocus("watermelon");
await pressKey("PageUp");
hasVirtualFocus("mango");
await pressKey("PageUp");
hasVirtualFocus("apple");
});
});
});
Loading