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

fix(Combobox): highlightMenuItem works correctly #3644

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

mshatikhin
Copy link
Member

Проблема

Решение

Ссылки

https://yt.skbkontur.ru/issue/IF-2215/ComboBox.-Regress.-Strannoe-povedenie-nachinaya-s-5.0

Чек-лист перед запросом ревью

  1. Добавлены тесты на все изменения
    ⬜ unit-тесты для логики
    ⬜ скриншоты для верстки и кросс-браузерности
    ⬜ нерелевантно

  2. Добавлена (обновлена) документация
    ⬜ styleguidist для пропов и примеров использования компонентов
    ⬜ jsdoc для утилит и хелперов
    ⬜ комментарии для неочевидных мест в коде
    ⬜ прочие инструкции (README.md, contributing.md и др.)
    ⬜ нерелевантно

  3. Изменения корректно типизированы
    ⬜ без использования any (см. PR 2856)
    ⬜ нерелевантно

  4. Прочее
    ⬜ все тесты и линтеры на CI проходят
    ⬜ в коде нет лишних изменений
    ⬜ заголовок PR кратко и доступно отражает суть изменений (он попадет в changelog)

@mshatikhin mshatikhin requested review from Copilot and zhzz April 7, 2025 11:32
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

packages/react-ui/internal/CustomComboBox/CustomComboBoxReducer.tsx:168

  • Consider using optional chaining (menu?.reset()) for consistency and to avoid potential runtime errors if menu is undefined.
menu.reset();

packages/react-ui/internal/CustomComboBox/CustomComboBoxReducer.tsx:180

  • Replacing menu.down() with menu.highlightItem(0) changes the behavior; please verify that this new behavior meets the intended UX requirements.
globalObject.requestAnimationFrame?.(() => menu?.highlightItem(0));

index = items.findIndex((x) => itemToValue(x) === itemToValue(value));
menu.highlightItem(index);
} else {
menu.reset();
Copy link
Member Author

Choose a reason for hiding this comment

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

Это чинит сценарий когда:
1 шаг чтото поискали, но не выбрали,
2 шаг ввели еще символы в поиск чтобы ничего не нашлось -- нажали Enter - будет выбран элемент который был первым на первом шаге

const queryTextDifferenceOfSavedValue = textValue !== valueString;
const needFocusToRefreshButton = requestStatus === ComboBoxRequestStatus.Failed;
if (queryTextDifferenceOfSavedValue || needFocusToRefreshButton) {
globalObject.requestAnimationFrame?.(() => menu?.highlightItem(0));
Copy link
Member Author

Choose a reason for hiding this comment

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

Это чинит сценарий:

Пишем в поиск Item. Выделился первый пункт в списке
Добавляем пробел в поиске. Выделился второй пункт. Почему?
Удаляем проблем в поиске. Выделился третий пункт. Почему?

@mshatikhin mshatikhin force-pushed the IF-2215_fix_combobox_highlightMenuItem branch from 65e7977 to 739953c Compare April 7, 2025 13:41
@mshatikhin mshatikhin force-pushed the IF-2215_fix_combobox_highlightMenuItem branch from 1366c4f to 2c72c85 Compare April 8, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant