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

Issue 1534: disabling collapse element new #1691

Closed
wants to merge 12 commits into from

Conversation

@Iogsotot Iogsotot changed the title Issue 1534 disabling collapse element new Issue 1534: disabling collapse element new Mar 28, 2024
Copy link
Contributor

@Iogsotot Iogsotot left a comment

Choose a reason for hiding this comment

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

image
Если нет PageObject вообще (пустой лист PO), то CaretDown тоже должен быть disabled

src/features/locators/components/LocatorListHeader.tsx Outdated Show resolved Hide resolved
src/features/locators/components/LocatorListHeader.tsx Outdated Show resolved Hide resolved
src/features/locators/components/LocatorListHeader.tsx Outdated Show resolved Hide resolved
@Iogsotot
Copy link
Contributor

Iogsotot commented Mar 29, 2024

В целом всё ок, нужны небольшие правки

я бы рекомендовала создать два состояния для каретки - isDisabled: true и isDisabled: false
isDisabled должно быть комплексным состоянием: стрелка повёрнута вниз, цвет цвет 000000 25% (Character/Disabled); это состояние наступает, когда нет PO, когда нет древовидной структуры локаторов

Ещё заметила какое-то дёрганье каретки, возможно это связано с useEffect и логикой внутри него

1534.1.mp4

@Alinochka07
Copy link
Contributor Author

Насчет дерганья каретки, обернула логику в useCallback. Вроде у меня нормально показывает.
И переделано в целом все как и было запрошено в комментах.

@Alinochka07 Alinochka07 requested a review from Iogsotot April 8, 2024 05:56
Copy link
Contributor

@Iogsotot Iogsotot left a comment

Choose a reason for hiding this comment

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

Судя по коду и задаче, header-collapse может иметь 3 состояния:
disabled - быть НЕкликабельным, смотреть вниз, цвет 000000 25% (Character/Disabled)
expanded - стрелка вверх, кликабельный, активный цвет
collapsed (или default) - стрелка вниз, кликабельный, активный цвет

Как можно упростить и организовать эти стили?
К примеру так:

  • выделить общее: "кликабельный" (курсор, евенты), активный цвет, - вынести эти стили в общий класс header-collapse

  • Затем можно вынести начальное (default) состояние для header-collapse - это состояние можно обозначить классом, а можно оставить "по умолчанию".
    - Если обозначить классом, то собрать все стили, отвечающие за default state в один класс (collapsed, как у тебя было сделано).
    - Можно не обозначать отдельным классом, и тогда все стили, относящиеся к default state, перенести в header-collapse

  • собрать все стили для disabled в одноименный класс (уже сделано, тут ок)

  • написать логику, по которой собирается класс для header-collapse (используя classnames, например)

  • отслеживать факторы, которые влияют на стейт коллапса и менять их по useEffect, props, useState или useMemo (выбрать наиболее оптимальный в данном кейсе вариант)

src/features/locators/styles/caretDown.less Outdated Show resolved Hide resolved
src/features/pageObjects/styles/caretDownExpand.less Outdated Show resolved Hide resolved
src/features/locators/components/LocatorListHeader.tsx Outdated Show resolved Hide resolved
src/features/locators/styles/caretDown.less Outdated Show resolved Hide resolved
@Iogsotot Iogsotot closed this May 29, 2024
@Iogsotot Iogsotot deleted the issue_1534-disabling-collapse-element branch May 29, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants