Skip to content

fix(MenuItem): use default cursor for nonselectable items#3606

Open
Funkicide wants to merge 5 commits intonextfrom
IF-2180_menuItem-pointer
Open

fix(MenuItem): use default cursor for nonselectable items#3606
Funkicide wants to merge 5 commits intonextfrom
IF-2180_menuItem-pointer

Conversation

@Funkicide
Copy link
Member

Проблема

При наведении на отключённые элементы курсор менялся на указатель, что неправильно. Курсор должен сообщать пользователю о состоянии элемента.

Решение

Добавил стиль с cursor:default на элемент с пропом isNotSelectable={true}

Ссылки

fix IF-2180

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

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

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

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

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

[styles.selected(this.theme)]: this.isSelected,
[styles.link(this.theme)]: !!link,
[this.getWithIconSizeClassName()]: Boolean(iconElement) || !!_enableIconPadding || this.context.enableIconPadding,
[styles.nonSelectable()]: !!this.props.isNotSelectable,
Copy link
Member

Choose a reason for hiding this comment

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

Предлагаю здесь заменить !!this.props.isNotSelectable на !!isNotSelectable, выше в методе эта переменная уже деструктуризирована из пропсов

Тут у меня еще возникла мысль, может стоит заодним убрать this.props и в 307, 314, 344 строчке.

Copy link
Member Author

@Funkicide Funkicide Mar 19, 2025

Choose a reason for hiding this comment

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

Предлагаю здесь заменить !!this.props.isNotSelectable на !!isNotSelectable, выше в методе эта переменная уже деструктуризирована из пропсов

Заменил.

Тут у меня еще возникла мысль, может стоит заодно убрать this.props и в 307, 314, 344 строчке.

Подлил некст, уже cделал это в другом ПР, оказывается.)

@SchwJ SchwJ self-requested a review March 19, 2025 09:12
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.

2 participants