Skip to content

Commit

Permalink
Combobox: Removed clear button, deprecated props clearButton/clearBut…
Browse files Browse the repository at this point in the history
…tonLabel, changed maxSelected to number. (#3433)

Co-authored-by: Ken <26967723+KenAJoh@users.noreply.github.com>
  • Loading branch information
HalvorHaugan and KenAJoh authored Dec 6, 2024
1 parent 5dedfa1 commit f720562
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 140 deletions.
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 staring with `--ac-combobox-clear`, deprecated props `clearButton`/`clearButtonLabel`.
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

0 comments on commit f720562

Please sign in to comment.