feat: add reposition and close scroll strategy (#DS-1920) - #1781
feat: add reposition and close scroll strategy (#DS-1920)#1781NikGurev wants to merge 10 commits into
Conversation
|
Visit the preview URL for this PR (updated for commit a340a7e): https://koobiq-next--prs-1781-ai6sk6pf.web.app (expires Thu, 30 Jul 2026 12:11:47 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new CDK-overlay scroll strategy in @koobiq/components/core that repositions overlays on scroll and emits a “hide” signal when the trigger/overlay goes out of bounds, making it usable both for built-in overlay-based components and custom overlay integrations.
Changes:
- Added
KbqHideOnScrollStrategy, config, hooks, andkbqHideOnScrollStrategyFactoryas public core API. - Exported the new strategy from
packages/components/core/public-api.ts. - Added Jest unit tests covering lifecycle, origin derivation, and out-of-bounds behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/public_api_guard/components/core.api.md | Adds the new scroll strategy types/functions to the public API snapshot. |
| packages/components/core/public-api.ts | Exports the new hide-on-scroll strategy from core’s public entrypoint. |
| packages/components/core/overlay/hide-on-scroll.strategy.ts | Implements the new scroll strategy, config, hooks, and factory. |
| packages/components/core/overlay/hide-on-scroll.strategy.spec.ts | Adds unit tests for the strategy’s lifecycle and boundary detection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enable(): void { | ||
| if (this.scrollSubscription) return; | ||
|
|
| .subscribe(() => { | ||
| this.overlayRef!.updatePosition(); | ||
|
|
||
| const isOutside = this.originElement | ||
| ? this._isOriginOutsideAncestors(this.originElement) | ||
| : this._isOverlayOutsideViewport(); | ||
|
|
||
| if (isOutside) { | ||
| this.ngZone.run(() => { | ||
| this.hideSubject.next(); | ||
| this.hooks?.onHide?.(); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
предлагаю не вводить новые суффиксы для имен файлов - hide-on-scroll-strategy.ts
https://angular.dev/style-guide#match-file-names-to-the-typescript-identifier-within
| if (!this.originElement) { | ||
| // FlexibleConnectedPositionStrategy stores the origin as a private field. | ||
| // Reading it here avoids requiring callers to pass originElement explicitly. | ||
| this.originElement = this.coerceOriginElement((overlayRef.getConfig().positionStrategy as any)?._origin); |
There was a problem hiding this comment.
это приватное поле, так что да
| ); | ||
| } | ||
|
|
||
| private _isOverlayOutsideViewport(): boolean { |
There was a problem hiding this comment.
у cdk есть метод, мб то что тебе нужно - isElementScrolledOutsideView
| * Pass `onHide` via the factory returned by `kbqHideOnScrollStrategyFactory` so the strategy | ||
| * calls it when the trigger scrolls out of bounds. | ||
| */ | ||
| export class KbqHideOnScrollStrategy implements ScrollStrategy { |
There was a problem hiding this comment.
текущее имя вводит в заблуждение, можно подумать, что стратегия скрывает оверлей при любом скролле, по сути это RepositionScrollStrategy + доп проверка на выход за границу видимости
может стоит отнаследовать RepositionScrollStrategy и сделать оверрайд метода закрытия?
Summary
Implemented separate scroll strategy that enhances Angular CDK scroll strategies.
Behavior is simple: hide if originElement is outside of view (cdkScrollable) and reposition otherwise.
It gets origin element from FlexibleConnectedPositionStrategy for built-in components using Angular Overlay, but gives the ability to provide in runtime for custom resolution.