Skip to content

MRK-4890: Компоненты: Добавить возможность закрытия тултипа при скролле (v8) #1307

Draft
andreykiselev92 wants to merge 24 commits into
masterfrom
v8/feature/MRK-4890
Draft

MRK-4890: Компоненты: Добавить возможность закрытия тултипа при скролле (v8) #1307
andreykiselev92 wants to merge 24 commits into
masterfrom
v8/feature/MRK-4890

Conversation

@andreykiselev92

Copy link
Copy Markdown
Collaborator

No description provided.

@harlamenko harlamenko marked this pull request as draft May 18, 2026 09:34
harlamenko added 11 commits June 7, 2026 15:09
Importing from the library's own public_api created a circular dependency
(public_api re-exports the tooltip service) and breaks the ng-packagr build.
Use the same relative path the directive and other specs already use.
…nted

Document the reason the custom strategies wrap a capture-phase document scroll
listener instead of reusing CDK's ScrollDispatcher-based strategies, and leave a
TODO to revisit.
…ate()

The EvoScrollStrategy -> concrete strategy switch was duplicated in the tooltip
service and the dropdown component. Move it into EvoScrollStrategyOptions.create()
so the union has a single owner, and replace the magic threshold with a named
constant in the tooltip service.
EvoScrollStrategyOptions is stateless, so provide it in root and drop the
duplicated providers entries from the tooltip directive, the dropdown component
and their specs.
It was only used by the element getter / listenScroll that were removed when the
dropdown switched to the shared scroll strategies; there are no subclasses.
EvoScrollStrategy and EvoScrollStrategyOptions both live in the common/scroll
barrel; import them from a single entry point instead of mixing a deep path with
the barrel.
CDK reads the scroll strategy only when the overlay opens, so the BehaviorSubject
+ async pipe bought nothing. Replace it with a plain field initialized in the
constructor (no longer relying on a parameter being assigned before a field
initializer) and drop the AsyncPipe import.
isOpen$ is a plain Subject; in the show test it emitted inside showTooltip before
the test subscribed, so first() never fired and the expectation was dead. Assert
service.hasAttached synchronously instead.
Replace the generic note with the concrete reason the custom stream exists: a
capture-phase document listener catches scroll from any container (CDK's bubble
listener + cdkScrollable registration misses unmarked overflow:auto blocks), and
the close threshold is measured from the trigger rect rather than the viewport
scroll position.
Replace the global capture-phase document listener with per-ancestor passive
listeners: walk the anchor's DOM ancestors, keep only real scroll containers
(getScrollableAncestors, ported from Floating UI's getOverflowAncestors), and
attach one passive listener per ancestor plus the window. This reacts only to the
few containers in the anchor's chain instead of every scroll on the page, still
needs no cdkScrollable markup, and re-discovers ancestors on each enable().

The anchor is passed as a lazy getOrigin() resolved on enable(), so the dropdown
origin input need not be set before the strategy is created. Unifies the close /
reposition params into EvoScrollStrategyParams.
Assert it collects overflow auto/scroll ancestors, skips overflow:visible, and
always appends the window last.
@harlamenko

Copy link
Copy Markdown
Collaborator

Вопрос 1

решит ли проблему если в ui-kit cdkscrollable повесить на оверлеи. в таком случае не нужно будет пользователям ui-kit вешать эти теги у себя?

Ответ

на оверлей вешать cdkScrollable бессмысленно; на контейнеры ui-kit — это частичное
решение, которое оставляет дыру в контейнерах приложения.


Альтернативная реализация

Брать паттерн Floating UI: обойти DOM-предков якоря, определить скролл-контейнеры по
getComputedStyle, и повесить по одному passive-слушателю scroll на каждый найденный
overflow-предок (+ window). Это «золотая середина»: никакой директивы у
потребителей и никакого глобального capture-листенера на все скроллы страницы. CDK эту задачу «из
коробки» не решает ни в одной версии — это подтверждено исходниками.

Где текущая логика с глобальным слушателем лучше:

  • Ноль подготовки при открытии. Новая при открытии один раз проходит вверх по дереву и читает
    getComputedStyle у предков (доли миллисекунды) — текущая этого НЕ делает.
  • Робастнее к динамике. Глобальный ловец видел вообще всё, поэтому если прокручиваемый контейнер появлялся вокруг якоря уже после открытия оверлея — текущая его ловит. Альтернативная фиксирует список предков в момент открытия и пере-обнаруживает его только на следующем открытии. Для тултипов/дропдаунов (короткоживущие, пересоздаются при каждом открытии) это почти не встречается, но формально это единственное измерение, где старый подход прощал больше.

Главное: как оверлей следит за прокруткой

Тултип/дропдаун должны «прилипать» к своему якорю (кнопке/иконке) и закрываться/переезжать, когда
пользователь крутит контейнер, в котором этот якорь лежит — модалку, сайдбар, таблицу.

Сейчас — один слушатель «на всё».
Пока оверлей открыт, библиотека вешала один слушатель на весь документ, который ловил
любую прокрутку где угодно на странице. На каждое событие код просыпался и проверял:
«это моё или нет?», лишнее отбрасывал.

  • 👍 ловит прокрутку любого контейнера без настройки;
  • 👎 дёргается на все прокрутки страницы (а их много: списки, автокомплиты, чужие модалки) —
    куча холостых срабатываний.

Альтернативно — датчики только там, где надо.
При открытии оверлея библиотека поднимается от якоря вверх по дереву и находит именно те
прокручиваемые контейнеры, внутри которых он лежит. И вешает отдельный лёгкий слушатель только
на них
(плюс на само окно). Реагирует лишь на ту прокрутку, которая реально может сдвинуть якорь.

  • 👍 тот же результат (следит за любым контейнером, никакой ручной разметки), но без реакции на
    посторонние прокрутки — обычно 2–5 точечных слушателей вместо одного «на всё».

Почему альтернативная (несколько слушателей) лучше

1. Меньше холостой работы.
текуща будит обработчик на каждую прокрутку на странице. Открыт оверлей → крутишь любой не
связанный с ним список/панель/модалку в другом углу экрана → обработчик всё равно срабатывает
(а потом отбрасывает событие как «не моё»). альтернативная вешает слушатели только
на 2–5 контейнеров, которые реально держат якорь — на чужие прокрутки она вообще не реагирует.

2. Каждое срабатывание у close/reposition стоит «чтения layout».
В стратегии close на каждом событии вызывается getBoundingClientRect() (а у текущего
repositionupdatePosition()). Это «дорогая» операция: браузер вынужден досчитать геометрию.
Под троттлингом это до ~60 раз/сек, пока что-то крутится. текущая делает эти 60 чтений/сек
даже когда крутят постороннее; альтернативная — ноль, если прокрутка не касается якоря.

3. Попутно исправлен баг дропдауна (это уже не перф, а поведение).
В ветке (до этой правки) дропдаун уходил в close без знания своего якоря → правило было
«закрыться на первую же прокрутку», а слушатель был глобальный → дропдаун закрывался при
прокрутке чего угодно на странице
, даже не связанного с ним. (В master до рефакторинга дропдаун
закрывался только если прокрученный элемент содержит его — то есть рефакторинг ветки случайно
расширил это до «любой скролл».) Новая версия слушает только предков якоря и знает якорь →
дропдаун закрывается только когда крутят контейнер, который реально его держит. Поведение
стало таким, каким и задумывалось.

4. Проще рассуждать.
«Один ловец на всё + фильтр» — это catch-all: надо в голове держать, что прилетит вообще всё и
нужно отфильтровать. «Слушатели ровно на нужных контейнерах» — сразу видно, на что реагируем.

Насколько плоха текущая

Честно — не катастрофа, рабочее первое приближение. Для тултипа разница вообще только в
эффективности: он и в старом варианте закрывался по порогу смещения (10px), то есть на чужих
прокрутках не закрывался — просто делал лишние getBoundingClientRect.

Порядок «ущерба» текущей версии:

  • Перф: до ~60 лишних чтений геометрии в секунду на каждый открытый оверлей, но только пока
    пользователь что-то прокручивает
    , и только если это «что-то» — не предок якоря. На современном
    железе это десятки микросекунд за чтение → в абсолюте немного. Бьёт заметнее на слабых
    устройствах, на огромном DOM и на долгоживущих дропдаунах.
  • Поведение: реальный минус был один — дропдаун закрывался от посторонних прокруток (см. п.3).
    Это видимый, но локальный баг.

Итог

альтернативная = тот же результат для пользователя, но реагирует точечно (только релевантные прокрутки),
не тратит геометрию впустую и заодно чинит «дропдаун закрывается от любого скролла». текущая не
сломана, а расточительна: лишние чтения layout на каждый посторонний скролл плюс этот один баг
дропдауна. Цена новой — крошечная подготовка при открытии и меньшая устойчивость к контейнерам,
появляющимся вокруг якоря уже после открытия
.


Вопрос 4

можно ли убрать дублирование из cdk?

Ответ

Что мы НЕ дублируем (берём у CDK как есть)

  • Позиционирование — целиком CDK (FlexibleConnectedPositionStrategy, OverlayPositionBuilder,
    overlayRef.updatePosition()). Своего тут ноль.
  • NoopScrollStrategy — используем напрямую (new NoopScrollStrategy()), не переписан.
  • Интерфейс ScrollStrategy, OverlayRef — это интеграция с CDK, а не дублирование.

Что мы по-прежнему дублируем из CDK

  • EvoRepositionScrollStrategy ≈ повторяет тело CDK RepositionScrollStrategy с autoClose:
    на скролл → updatePosition(), ушёл из вида → detach(). Перекрытие высокое.
  • EvoCloseScrollStrategy — каркас (attach/detach/enable/disable + закрытие на скролл)
    повторяет CDK CloseScrollStrategy. Но ядро порога другое: мы меряем смещение
    getBoundingClientRect() триггера по обеим осям, а CDK — только вертикальный скролл вьюпорта
    (который при скролле вложенного контейнера вообще не меняется). Здесь дублируется обвязка, а сама
    проверка у нас осознанно своя и для вложенных контейнеров — более правильная.

Эту правку per-ancestor не трогала — тела reposition/close остались как были. То есть на
дублирование CDK она повлияла нейтрально.

Что в принципе НЕ из CDK

  • getScrollableAncestors + createScrollStream — обход DOM-предков по getComputedStyle.
    Этого в CDK нет вообще: ScrollDispatcher.getAncestorScrollContainers() возвращает только
    контейнеры, помеченные cdkScrollable, и по дереву не ходит. Так что это не дубль CDK, а дубль
    Floating UI (getOverflowAncestors, осознанный порт). Именно этот кусок и оправдывает
    существование всего кастомного модуля.

Можно ли убрать дублирование CDK

Частично — да, и это как раз то, на что оставлен TODO(MRK-4890). Раз мы теперь сами находим
предков, можно было бы регистрировать их в ScrollDispatcher и переиспользовать штатную CDK
RepositionScrollStrategy — тогда EvoRepositionScrollStrategy удаляется.

Но полностью не выйдет:

  • close так не делегируется без регресса — у CDK порог завязан на скролл вьюпорта, а нам нужен
    rect-based по триггеру (иначе во вложенных контейнерах перестанет работать). То есть тело close
    всё равно останется своим.
  • Регистрация предков в ScrollDispatcher требует ручного создания CdkScrollable-обёрток на
    каждый контейнер с управлением их жизненным циклом — это своя обвязка взамен текущей.

Итог

  • Дублирование тел CDK-стратегий (reposition полностью, каркас close) — осталось, эта правка
    его не убирала.

  • Полностью переиспользовать CDK мешает один пункт: rect-based порог close для вложенных
    контейнеров, которого у CDK нет.

  • Новый код добавил не дубль CDK, а порт Floating UI (getScrollableAncestors) — то единственное,
    чего CDK не умеет.

    Заодно почистили (по код-ревью)

Что Было Стало
Импорт в сервисе тултипа тянул зависимость «сам из себя» через общий публичный файл (риск циклической зависимости и поломки сборки) прямой относительный импорт
Выбор стратегии (close/reposition/noop) один и тот же switch продублирован в тултипе и дропдауне один метод create() на оба
Сервис стратегий подключался отдельно в каждом компоненте (дубли) регистрируется глобально один раз
Хранение стратегии в дропдауне «поток» (BehaviorSubject + async-пайп), хотя CDK читает стратегию один раз при открытии обычное поле — проще, без лишней обвязки
Лишняя инъекция в дропдауне висела неиспользуемая зависимость удалена
Один тест проверка была «пустой» — никогда не выполнялась заменена на реальную
Комментарии общая формулировка объяснено, почему не берём штатные CDK-стратегии (+ ссылка на задачу)

…ep dropdown close fix

Drop the per-ancestor discovery (getScrollableAncestors) and restore the single
capture-phase document scroll listener. Keep the dropdown fix: the close strategy
still receives the anchor via getOrigin and decides to close from the anchor's
getBoundingClientRect() movement, so the dropdown closes only when its own
container actually moves it, not on any page scroll. Reposition no longer takes
params.
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