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

Combobox: Removed clear button, deprecated props clearButton/clearButtonLabel, changed maxSelected to number. #3433

Merged
merged 3 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions .changeset/quick-seas-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@navikt/aksel-stylelint": minor
"@navikt/ds-react": minor
"@navikt/ds-css": minor
---

Combobox: Removed clear button, removed tokens `--ac-combobox-clear-icon*`, deprecated props `clearButton`/`clearButtonLabel`.
HalvorHaugan marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions .changeset/strong-carpets-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@navikt/ds-react": minor
---

Combobox: Changed prop `maxSelected` to number
4 changes: 4 additions & 0 deletions @navikt/aksel-stylelint/src/deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ export const deprecations: DeprecatedList = [
classes: ["navds-list--nested", "navds-list__item-content"],
message: "Removed in v7.1.1",
},
{
classes: ["navds-combobox__button-clear"],
message: "Removed in v7.8.0",
},
];
21 changes: 0 additions & 21 deletions @navikt/core/css/darkside/form/combobox.darkside.css
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
}
}

.navds-combobox__button-clear svg,
.navds-combobox__button-toggle-list svg,
.navds-combobox__list svg {
width: var(--__axc-combobox-icon-size);
Expand Down Expand Up @@ -203,19 +202,6 @@
}
}

.navds-combobox__button-clear {
border-radius: var(--ax-border-radius-medium);
color: var(--ax-text-subtle);
display: flex;
justify-content: center;
align-items: center;
cursor: pointer;
background: none;
border: none;
font-size: 1rem;
padding: 0;
}

.navds-combobox__input::-webkit-search-cancel-button {
display: none;
}
Expand All @@ -231,10 +217,7 @@
border: none;
font-size: 1rem;
padding: 0;
}

.navds-combobox__button-clear,
.navds-combobox__button-toggle-list {
&:hover {
color: var(--ax-text-accent);

Expand Down Expand Up @@ -405,10 +388,6 @@
}

@media (forced-colors: active) {
.navds-combobox__button-clear:hover {
color: highlight;
}

.navds-combobox__wrapper-inner:has(.navds-combobox__input:focus-visible) {
outline-color: highlight;
}
Expand Down
24 changes: 2 additions & 22 deletions @navikt/core/css/form/combobox.css
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
border-color: var(--a-border-subtle);
}

.navds-combobox__button-clear svg,
.navds-combobox__button-toggle-list svg,
.navds-combobox__list svg {
width: var(--__ac-combobox-icon-size);
Expand Down Expand Up @@ -195,19 +194,6 @@
}
}

.navds-combobox__button-clear {
border-radius: var(--a-border-radius-medium);
color: var(--ac-combobox-clear-icon, var(--a-text-subtle));
display: flex;
justify-content: center;
align-items: center;
cursor: pointer;
background: none;
border: none;
font-size: 1rem;
padding: 0;
}

.navds-combobox__input::-webkit-search-cancel-button {
display: none;
}
Expand All @@ -225,14 +211,12 @@
padding: 0;
}

.navds-combobox__button-clear:active:hover,
.navds-combobox__button-toggle-list:active:hover {
color: var(--ac-combobox-clear-icon-active, var(--a-text-action));
color: var(--a-text-action);
}

.navds-combobox__button-clear:hover,
.navds-combobox__button-toggle-list:hover {
color: var(--ac-combobox-clear-icon-hover, var(--a-text-action-selected));
color: var(--a-text-action-selected);
}

.navds-combobox__button-toggle-list:focus-visible {
Expand Down Expand Up @@ -406,10 +390,6 @@
}

@media (forced-colors: active) {
.navds-combobox__button-clear:hover {
color: highlight;
}

.navds-combobox__wrapper-inner:has(.navds-combobox__input:focus-visible) {
outline-color: highlight;
}
Expand Down
3 changes: 0 additions & 3 deletions @navikt/core/css/tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,6 @@
"--ac-search-error-border": "--a-border-danger"
},
"combobox": {
"--ac-combobox-clear-icon": "--a-text-subtle",
"--ac-combobox-clear-icon-hover": "--a-text-action-selected",
"--ac-combobox-clear-icon-active": "--a-text-action",
"--ac-combobox-list-bg": "--a-surface-default",
"--ac-combobox-list-text": "--a-text-default",
"--ac-combobox-list-border-color": "--a-border-divider",
Expand Down
12 changes: 6 additions & 6 deletions @navikt/core/react/src/form/combobox/ComboboxWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ const ComboboxWrapper = ({
const wrapperRef = useRef<HTMLDivElement | null>(null);
const [hasFocusWithin, setHasFocusWithin] = useState(false);

function onFocusInsideWrapper(e) {
function onFocusInsideWrapper(event: React.FocusEvent<HTMLDivElement>) {
if (
!wrapperRef.current?.contains(e.relatedTarget) &&
toggleOpenButtonRef?.current !== e.target
!wrapperRef.current?.contains(event.relatedTarget) &&
toggleOpenButtonRef?.current !== event.target
) {
toggleIsListOpen(true);
setHasFocusWithin(true);
}
}

function onBlurWrapper(e) {
if (!wrapperRef.current?.contains(e.relatedTarget)) {
function onBlurWrapper(event: React.FocusEvent<HTMLDivElement>) {
if (!wrapperRef.current?.contains(event.relatedTarget)) {
toggleIsListOpen(false);
setHasFocusWithin(false);
clearInput(e);
clearInput(event);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ const FilteredOptions = () => {
const { maxSelected } = useSelectedOptionsContext();

const shouldRenderNonSelectables =
maxSelected?.isLimitReached || // Render maxSelected message
maxSelected.isLimitReached || // Render maxSelected message
isLoading || // Render loading message
(!isLoading && filteredOptions.length === 0 && !allowNewValues); // Render no hits message

const shouldRenderFilteredOptionsList =
(allowNewValues && isValueNew && !maxSelected?.isLimitReached) || // Render add new option
(allowNewValues && isValueNew && !maxSelected.isLimitReached) || // Render add new option
filteredOptions.length > 0; // Render filtered options

return (
Expand All @@ -45,7 +45,7 @@ const FilteredOptions = () => {
>
{shouldRenderNonSelectables && (
<div className="navds-combobox__list_non-selectables" role="status">
{maxSelected?.isLimitReached && <MaxSelectedMessage />}
{maxSelected.isLimitReached && <MaxSelectedMessage />}
{isLoading && <LoadingMessage />}
{!isLoading && filteredOptions.length === 0 && !allowNewValues && (
<NoSearchHitsMessage />
Expand All @@ -60,7 +60,7 @@ const FilteredOptions = () => {
role="listbox"
className="navds-combobox__list-options"
>
{isValueNew && !maxSelected?.isLimitReached && allowNewValues && (
{isValueNew && !maxSelected.isLimitReached && allowNewValues && (
<AddNewOption />
)}
{filteredOptions.map((option) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const FilteredOptionsItem = ({ option }: { option: ComboboxOption }) => {
const [start, highlight, end] = useTextHighlight(option.label, searchTerm);

const isDisabled = (_option: ComboboxOption) =>
maxSelected?.isLimitReached && !isInList(_option.value, selectedOptions);
maxSelected.isLimitReached && !isInList(_option.value, selectedOptions);

return (
<li
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,7 @@ const MaxSelectedMessage = () => {
inputProps: { id },
} = useInputContext();
const { maxSelected, selectedOptions } = useSelectedOptionsContext();
const translate = useI18n(
"Combobox",
maxSelected?.message ? { maxSelected: maxSelected.message } : undefined,
);

if (!maxSelected) {
return null;
}
const translate = useI18n("Combobox");

return (
<div
Expand All @@ -25,7 +18,7 @@ const MaxSelectedMessage = () => {
>
{translate("maxSelected", {
selected: selectedOptions.length,
limit: maxSelected.limit,
limit: maxSelected.limit || 0,
})}
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@ const FilteredOptionsProvider = ({
}
virtualFocus.resetFocus();
if (newState ?? !isInternalListOpen) {
setHideCaret(!!maxSelected?.isLimitReached);
setHideCaret(maxSelected.isLimitReached);
}
setInternalListOpen((oldState) => newState ?? !oldState);
},
[
virtualFocus,
maxSelected?.isLimitReached,
maxSelected.isLimitReached,
isInternalListOpen,
setHideCaret,
disabled,
Expand Down Expand Up @@ -178,7 +178,7 @@ const FilteredOptionsProvider = ({
}
}
const maybeMaxSelectedOptionsId =
maxSelected?.isLimitReached &&
maxSelected.isLimitReached &&
filteredOptionsUtils.getMaxSelectedOptionsId(id);

return (
Expand All @@ -188,7 +188,7 @@ const FilteredOptionsProvider = ({
}, [
isListOpen,
isLoading,
maxSelected?.isLimitReached,
maxSelected.isLimitReached,
value,
partialAriaDescribedBy,
shouldAutocomplete,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const InputProvider = ({ children, value: props }: Props) => {
);

const clearInput = useCallback(
(event: React.PointerEvent | React.KeyboardEvent | React.MouseEvent) => {
(event: React.PointerEvent | React.KeyboardEvent | React.FocusEvent) => {
onClear?.(event);
externalOnChange?.("");
setInternalValue("");
Expand Down
2 changes: 1 addition & 1 deletion @navikt/core/react/src/form/combobox/Input/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
value={value}
onBlur={composeEventHandlers(onBlur, virtualFocus.resetFocus)}
onClick={() => {
setHideCaret(!!maxSelected?.isLimitReached);
setHideCaret(maxSelected.isLimitReached);
value !== searchTerm && onChange(value);
}}
onInput={onChangeHandler}
Expand Down
27 changes: 4 additions & 23 deletions @navikt/core/react/src/form/combobox/Input/InputController.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
/* eslint-disable jsx-a11y/no-static-element-interactions */
import cl from "clsx";
import React, { forwardRef } from "react";
import { XMarkIcon } from "@navikt/aksel-icons";
import { useMergeRefs } from "../../../util/hooks";
import { useI18n } from "../../../util/i18n/i18n.context";
import { useFilteredOptionsContext } from "../FilteredOptions/filteredOptionsContext";
import SelectedOptions from "../SelectedOptions/SelectedOptions";
import { useSelectedOptionsContext } from "../SelectedOptions/selectedOptionsContext";
Expand All @@ -28,7 +26,9 @@ export const InputController = forwardRef<
>
>((props, ref) => {
const {
clearButton = true,
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- Remove when prop has been removed from ComboboxProps.
clearButton,
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- Remove when prop has been removed from ComboboxProps.
clearButtonLabel,
toggleListButton = true,
inputClassName,
Expand All @@ -37,10 +37,8 @@ export const InputController = forwardRef<
} = props;

const {
clearInput,
focusInput,
inputProps,
value,
size = "medium",
inputRef,
toggleOpenButtonRef,
Expand All @@ -52,11 +50,6 @@ export const InputController = forwardRef<

const mergedInputRef = useMergeRefs(inputRef, ref);

const translate = useI18n(
"Combobox",
clearButtonLabel ? { clear: clearButtonLabel } : undefined,
);

return (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events
<div
Expand Down Expand Up @@ -86,19 +79,7 @@ export const InputController = forwardRef<
/>
</SelectedOptions>
)}
<div>
{value && clearButton && (
<div
onClick={clearInput}
className="navds-combobox__button-clear"
aria-hidden
title={translate("clear")}
>
<XMarkIcon />
</div>
)}
{toggleListButton && <ToggleListButton ref={toggleOpenButtonRef} />}
</div>
{toggleListButton && <ToggleListButton ref={toggleOpenButtonRef} />}
</div>
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type SelectedOptionsContextValue = {
removeSelectedOption: (option: ComboboxOption) => void;
prevSelectedOptions?: ComboboxOption[];
selectedOptions: ComboboxOption[];
maxSelected?: ComboboxProps["maxSelected"] & { isLimitReached: boolean };
maxSelected: { limit: number | undefined; isLimitReached: boolean };
setSelectedOptions: (any) => void;
toggleOption: (
option: ComboboxOption,
Expand Down Expand Up @@ -101,14 +101,17 @@ const SelectedOptionsProvider = ({
[customOptions, onToggleSelected, removeCustomOption],
);

const maxSelectedLimit =
typeof maxSelected === "object" ? maxSelected.limit : maxSelected;
const isLimitReached =
(!!maxSelected?.limit && selectedOptions.length >= maxSelected.limit) ||
(!isMultiSelect && selectedOptions.length > 0);
!!maxSelectedLimit && selectedOptions.length >= maxSelectedLimit;
const newHideCaret =
isLimitReached || (!isMultiSelect && selectedOptions.length > 0);

// biome-ignore lint/correctness/useExhaustiveDependencies: We explicitly want to run this effect when selectedOptions changes to match the view with the selected options.
useEffect(() => {
setHideCaret(isLimitReached);
}, [isLimitReached, selectedOptions, setHideCaret]);
setHideCaret(newHideCaret);
}, [newHideCaret, selectedOptions, setHideCaret]);

const toggleOption = useCallback(
(
Expand Down Expand Up @@ -142,8 +145,8 @@ const SelectedOptionsProvider = ({
selectedOptions,
setSelectedOptions,
toggleOption,
maxSelected: maxSelected && {
...maxSelected,
maxSelected: {
limit: maxSelectedLimit,
isLimitReached,
},
};
Expand Down
Loading
Loading