Skip to content

Add 'Self service permissions' main page #1153

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.

Sign up for GitHub

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

Open
carma12 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add 'Self service permissions' main page #1153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
346 changes: 346 additions & 0 deletions src/components/TypeAheadWithCheckbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,346 @@
import React, { useEffect, useRef, useState } from "react";
import {
Select,
SelectOption,
SelectList,
SelectOptionProps,
MenuToggle,
MenuToggleElement,
TextInputGroup,
TextInputGroupMain,
TextInputGroupUtilities,
Button,
} from "@patternfly/react-core";
import { CloseIcon } from "@patternfly/react-icons";

type CreationProps<T> = {
// Whenever this property changes, the options will be reset
onChangeTarget: T;
};

type TypeAheadWithCheckboxProps<T> = {
id: string;
dataCy: string;
options: SelectOptionProps[];
selected: string[];
setSelected: (selected: string[]) => void;
creationProps?: CreationProps<T>;
};

const NO_RESULTS = "no results";
const CREATE_NEW = "create";

export const TypeAheadWithCheckbox = <T,>({

@sourcery-ai sourcery-ai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (complexity): Consider simplifying this component by tightening state derivation, extracting shared selection logic, and optionally moving keyboard navigation into a small hook.

You can noticeably reduce complexity without changing behavior by tightening up state management and extracting a couple of small helpers.

1. Remove setState from render for localOptions

Instead of mutating state during render based on creationProps.onChangeTarget, track that via useEffect (or a key reset).

Current:

const [localOptions, setLocalOptions] =
  useState<SelectOptionProps[]>(options);

const [previousTarget, setPreviousTarget] = useState<T | null>(null);
const allowCreation = creationProps !== undefined;

if (allowCreation && previousTarget !== creationProps?.onChangeTarget) {
  setPreviousTarget(creationProps.onChangeTarget);
  setLocalOptions(options);
  setInputValue("");
}

Suggested:

const [localOptions, setLocalOptions] =
  useState<SelectOptionProps[]>(options);

const allowCreation = creationProps !== undefined;

useEffect(() => {
  if (!allowCreation) return;
  setLocalOptions(options);
  setInputValue("");
}, [allowCreation, creationProps?.onChangeTarget, options]);

Or, if acceptable, drive this from a key on the component where it’s used instead of internal bookkeeping.

2. Treat availableOptions as derived state

You don’t need useEffect + mutable newSelectOptions. A useMemo keeps behavior but removes imperative mutation and the eslint suppression.

Current:

const [availableOptions, setAvailableOptions] =
  useState<SelectOptionProps[]>(localOptions);

useEffect(() => {
  let newSelectOptions: SelectOptionProps[] = localOptions;

  // ... mutate newSelectOptions ...

  setAvailableOptions(newSelectOptions);
}, [inputValue, localOptions]);

Suggested:

const availableOptions = React.useMemo(() => {
  let newSelectOptions: SelectOptionProps[] = localOptions;

  if (inputValue) {
    newSelectOptions = localOptions.filter((menuItem) =>
      String(menuItem.children).toLowerCase().includes(inputValue.toLowerCase()),
    );

    if (allowCreation) {
      if (!localOptions.some((option) => option.value === inputValue)) {
        newSelectOptions = [
          ...newSelectOptions,
          {
            children: `Create new option "${inputValue}"`,
            value: CREATE_NEW,
            "data-cy": `${dataCy}-create-new-option`,
          },
        ];
      }
    }

    if (newSelectOptions.length === 0) {
      newSelectOptions = [
        {
          "data-cy": `${dataCy}-no-results`,
          isAriaDisabled: true,
          children: `No results found for "${inputValue}"`,
          value: NO_RESULTS,
          hasCheckbox: false,
        },
      ];
    }
  }

  return newSelectOptions;
}, [localOptions, inputValue, allowCreation, dataCy]);

Then drop availableOptions’s state and setter:

// remove:
// const [availableOptions, setAvailableOptions] = useState(...);

3. Factor selection toggling into a helper

onSelect has duplicated “toggle in array” logic (for both CREATE_NEW and normal values). A small helper clarifies behavior and shortens the function.

Current snippets:

setSelected(
  selected.includes(inputValue)
    ? selected.filter((selection) => selection !== inputValue)
    : [...selected, inputValue]
);

setSelected(
  selected.includes(value)
    ? selected.filter((selection) => selection !== value)
    : [...selected, value]
);

Suggested:

const toggleSelection = (current: string[], value: string) =>
  current.includes(value)
    ? current.filter((selection) => selection !== value)
    : [...current, value];

const onSelect = (value: string) => {
  if (!value || value === NO_RESULTS) {
    textInputRef.current?.focus();
    return;
  }

  if (value === CREATE_NEW) {
    if (!availableOptions.some((item) => item.value === inputValue)) {
      setLocalOptions([
        ...localOptions,
        {
          value: inputValue,
          children: inputValue,
          "data-cy": `${dataCy}-${inputValue}-create-new-option`,
        },
      ]);
    }
    setSelected(toggleSelection(selected, inputValue));
    resetActiveAndFocusedItem();
  } else {
    setSelected(toggleSelection(selected, value));
  }

  textInputRef.current?.focus();
};

4. Consider extracting keyboard navigation into a small hook

The focusedItemIndex/activeItemId logic is correct but dense. You can move it into a hook without changing behavior:

function useListKeyboardNavigation(options: SelectOptionProps[]) {
  const [focusedItemIndex, setFocusedItemIndex] = useState<number | null>(null);
  const [activeItemId, setActiveItemId] = useState<string | null>(null);

  const setActiveAndFocusedItem = (index: number) => {
    setFocusedItemIndex(index);
    const focusedItem = options[index];
    setActiveItemId(`select-multi-typeahead-${String(focusedItem.value).replace(" ", "-")}`);
  };

  const reset = () => {
    setFocusedItemIndex(null);
    setActiveItemId(null);
  };

  return {
    focusedItemIndex,
    activeItemId,
    setActiveAndFocusedItem,
    reset,
  };
}

Then the main component just calls this hook and keeps handleMenuArrowKeys/onInputKeyDown shorter:

const {
  focusedItemIndex,
  activeItemId,
  setActiveAndFocusedItem,
  reset: resetActiveAndFocusedItem,
} = useListKeyboardNavigation(availableOptions);

This keeps all current behavior but makes the main component easier to follow.

id,
dataCy,
options,
selected,
setSelected,
creationProps,
}: TypeAheadWithCheckboxProps<T>) => {
// This is to allow mutations to options property
const [localOptions, setLocalOptions] =
useState<SelectOptionProps[]>(options);
// This one is actually shown, it can contain Create New options and no results
const [availableOptions, setAvailableOptions] =
useState<SelectOptionProps[]>(localOptions);
const [isOpen, setIsOpen] = useState(false);
const [inputValue, setInputValue] = useState<string>("");
const [focusedItemIndex, setFocusedItemIndex] = useState<number | null>(null);
const [activeItemId, setActiveItemId] = useState<string | null>(null);
const textInputRef = useRef<HTMLInputElement>(undefined);

const [previousTarget, setPreviousTarget] = useState<T | null>(null);
const allowCreation = creationProps !== undefined;

if (allowCreation && previousTarget !== creationProps?.onChangeTarget) {
setPreviousTarget(creationProps.onChangeTarget);
Comment on lines +56 to +57

@sourcery-ai sourcery-ai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): Avoid calling setState during render when reacting to creationProps changes

This code calls setPreviousTarget, setLocalOptions, and setInputValue during render whenever creationProps?.onChangeTarget changes, which can cause render loops and violates React’s state update rules. Move this logic into a useEffect that depends on allowCreation, options, and creationProps?.onChangeTarget so the reset happens after render instead of during it.

setLocalOptions(options);
setInputValue("");
}

useEffect(() => {
let newSelectOptions: SelectOptionProps[] = localOptions;

// Filter menu items based on the text input value when one exists
if (inputValue) {
newSelectOptions = localOptions.filter((menuItem) =>
String(menuItem.children)
.toLowerCase()
.includes(inputValue.toLowerCase())
);

// If no option matches the filter exactly, display creation option
if (allowCreation) {
if (!localOptions.some((option) => option.value === inputValue)) {
newSelectOptions = [
...newSelectOptions,
{
children: `Create new option "${inputValue}"`,
value: CREATE_NEW,
"data-cy": `${dataCy}-create-new-option`,
},
];
}
}

// When no options are found after filtering, display 'No results found'
if (newSelectOptions.length === 0) {
newSelectOptions = [
{
"data-cy": `${dataCy}-no-results`,
isAriaDisabled: true,
children: `No results found for "${inputValue}"`,
value: NO_RESULTS,
hasCheckbox: false,
},
];
}
}

// This sucks, but we're forced to, due to how isOpenCallback
// works, with memo the we onOpenChange is fired
// eslint-disable-next-line @eslint-react/hooks-extra/no-direct-set-state-in-use-effect
setAvailableOptions(newSelectOptions);
// We mutate options, if we don't, we end up with the same issue as above...
}, [inputValue, localOptions]);

const placeholder = `${selected.length} item${selected.length !== 1 ? "s" : ""} selected`;

const createItemId = (value: string) =>
`select-multi-typeahead-${value.replace(" ", "-")}`;

const setActiveAndFocusedItem = (itemIndex: number) => {
setFocusedItemIndex(itemIndex);
const focusedItem = availableOptions[itemIndex];
setActiveItemId(createItemId(focusedItem.value));
};

const resetActiveAndFocusedItem = () => {
setFocusedItemIndex(null);
setActiveItemId(null);
};

const closeMenu = () => {
setIsOpen(false);
resetActiveAndFocusedItem();
};

const onInputClick = () => {
if (!isOpen) {
setIsOpen(true);
} else if (!inputValue) {
closeMenu();
}
};

const handleMenuArrowKeys = (key: string) => {
let indexToFocus = 0;

if (!isOpen) {
setIsOpen(true);
}

if (availableOptions.every((option) => option.isDisabled)) {
return;
}

if (key === "ArrowUp") {
// When no index is set or at the first index, focus to the last, otherwise decrement focus index
if (focusedItemIndex === null || focusedItemIndex === 0) {
indexToFocus = availableOptions.length - 1;
} else {
indexToFocus = focusedItemIndex - 1;
}

// Skip disabled options
while (availableOptions[indexToFocus].isDisabled) {
indexToFocus--;
if (indexToFocus === -1) {
indexToFocus = availableOptions.length - 1;
}
}
}

if (key === "ArrowDown") {
// When no index is set or at the last index, focus to the first, otherwise increment focus index
if (
focusedItemIndex === null ||
focusedItemIndex === availableOptions.length - 1
) {
indexToFocus = 0;
} else {
indexToFocus = focusedItemIndex + 1;
}

// Skip disabled options
while (availableOptions[indexToFocus].isDisabled) {
indexToFocus++;
if (indexToFocus === availableOptions.length) {
indexToFocus = 0;
}
}
}

setActiveAndFocusedItem(indexToFocus);
};

const onInputKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
const focusedItem =
focusedItemIndex !== null ? availableOptions[focusedItemIndex] : null;

switch (event.key) {
case "Enter":
if (
isOpen &&
focusedItem &&
focusedItem.value !== NO_RESULTS &&
!focusedItem.isAriaDisabled
) {
onSelect(focusedItem.value);
}

if (!isOpen) {
setIsOpen(true);
}

break;
case "ArrowUp":
case "ArrowDown":
event.preventDefault();
handleMenuArrowKeys(event.key);
break;
}
};

const onToggleClick = () => {
setIsOpen(!isOpen);
textInputRef?.current?.focus();
};

const onTextInputChange = (
_event: React.FormEvent<HTMLInputElement>,
value: string
) => {
setInputValue(value);
if (value !== "" && !isOpen) setIsOpen(true);
resetActiveAndFocusedItem();
};

const onSelect = (value: string) => {
if (value && value !== NO_RESULTS) {
if (value === CREATE_NEW) {
if (!availableOptions.some((item) => item.value === inputValue)) {
setLocalOptions([
...localOptions,
{
value: inputValue,
children: inputValue,
"data-cy": `${dataCy}-${inputValue}-create-new-option`,
},
]);
}
setSelected(
selected.includes(inputValue)
? selected.filter((selection) => selection !== inputValue)
: [...selected, inputValue]
);
resetActiveAndFocusedItem();
} else {
setSelected(
selected.includes(value)
? selected.filter((selection) => selection !== value)
: [...selected, value]
);
}
}

textInputRef.current?.focus();
};

const onClearButtonClick = () => {
setSelected([]);
setInputValue("");
resetActiveAndFocusedItem();
textInputRef?.current?.focus();
};

const toggle = (toggleRef: React.Ref<MenuToggleElement>) => (
<MenuToggle
data-cy={`${dataCy}-multi-typeahead-checkbox-menu-toggle`}
variant="typeahead"
aria-label="Multi typeahead checkbox menu toggle"
onClick={onToggleClick}
innerRef={toggleRef}
isExpanded={isOpen}
isFullWidth
>
<TextInputGroup isPlain>
<TextInputGroupMain
value={inputValue}
onClick={onInputClick}
onChange={onTextInputChange}
onKeyDown={onInputKeyDown}
id={`${id}-multi-typeahead-select-checkbox-input`}
autoComplete="off"
innerRef={textInputRef}
placeholder={placeholder}
{...(activeItemId && { "aria-activedescendant": activeItemId })}
role="combobox"
isExpanded={isOpen}
aria-controls={`${id}-select-multi-typeahead-checkbox-listbox`}
/>
<TextInputGroupUtilities
{...(selected.length === 0 ? { style: { display: "none" } } : {})}
>
<Button
data-cy={`${dataCy}-multi-typeahead-checkbox-clear-button`}
variant="plain"
onClick={onClearButtonClick}
aria-label="Clear input value"
icon={<CloseIcon />}
/>
</TextInputGroupUtilities>
</TextInputGroup>
</MenuToggle>
);

return (
<Select
data-cy={`${dataCy}-multi-typeahead-checkbox-select`}
role="menu"
id={`${id}-multi-typeahead-checkbox-select`}
isOpen={isOpen}
selected={selected}
onSelect={(_event, selection) => onSelect(selection as string)}
onOpenChange={(isOpen) => {
if (!isOpen) closeMenu();
}}
toggle={toggle}
variant="typeahead"
isScrollable
maxMenuHeight="300px"
>
<SelectList
isAriaMultiselectable
id={`${id}-select-multi-typeahead-checkbox-listbox`}
>
{availableOptions.map((option, index) => (
<SelectOption
key={option.value || option.children}
{...(!option.isDisabled &&
!option.isAriaDisabled && { hasCheckbox: true })}
isSelected={selected.includes(option.value)}
isFocused={focusedItemIndex === index}
className={option.className}
id={`${id}-${createItemId(option.value)}`}
{...option}
ref={null}
>
{option.children}
</SelectOption>
))}
</SelectList>
</Select>
);
};
Loading
Loading