Skip to content

Feat/tooltip/configure scroll strategy#1998

Merged
chintankavathia merged 2 commits intomainfrom
feat/tooltip/configure-scroll-strategy
May 7, 2026
Merged

Feat/tooltip/configure scroll strategy#1998
chintankavathia merged 2 commits intomainfrom
feat/tooltip/configure-scroll-strategy

Conversation

@spike-rabbit
Copy link
Copy Markdown
Member

@spike-rabbit spike-rabbit commented May 4, 2026

Allows configuring the scrollStrategy for tooltips and popover.
This follows the same approach we already have in the typeahead.

Closes https://code.siemens.com/simpl/simpl-element/-/work_items/2703


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for custom CDK scroll strategies in popover and tooltip components by updating the overlay helper and respective directives and services. Feedback highlights a potential memory leak in the popover directive due to missing overlay disposal and suggests adding SSR safety checks. Furthermore, several newly added test components require the OnPush change detection strategy to align with the project's style guide.

Comment thread projects/element-ng/popover/si-popover.directive.ts
Comment thread projects/element-ng/popover/si-popover.directive.spec.ts Outdated
Comment thread projects/element-ng/popover/si-popover.directive.spec.ts
Comment thread projects/element-ng/tooltip/si-tooltip.directive.spec.ts
@spike-rabbit spike-rabbit force-pushed the feat/tooltip/configure-scroll-strategy branch 4 times, most recently from 089982e to ad2bd40 Compare May 4, 2026 12:34
@spike-rabbit spike-rabbit marked this pull request as ready for review May 4, 2026 12:48
@spike-rabbit spike-rabbit requested review from a team as code owners May 4, 2026 12:48
Comment thread projects/element-ng/tooltip/si-tooltip.directive.ts
Comment thread projects/element-ng/tooltip/si-tooltip.directive.spec.ts Outdated
Comment thread projects/element-ng/popover/si-popover.directive.spec.ts Outdated
@spike-rabbit spike-rabbit force-pushed the feat/tooltip/configure-scroll-strategy branch 2 times, most recently from 71eb5ee to 9a644a0 Compare May 5, 2026 15:18
Allows consumer to define a `ScrollStrategy` for the tooltip overlay.
Allows consumer to define a `ScrollStrategy` for the popover overlay.
@spike-rabbit spike-rabbit force-pushed the feat/tooltip/configure-scroll-strategy branch from 9a644a0 to 43232c4 Compare May 5, 2026 15:23
Copy link
Copy Markdown
Member

@chintankavathia chintankavathia left a comment

Choose a reason for hiding this comment

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

👍

@chintankavathia chintankavathia enabled auto-merge May 6, 2026 11:24
Copy link
Copy Markdown
Member

@kfenner kfenner left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chintankavathia chintankavathia added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit d642809 May 7, 2026
17 checks passed
@chintankavathia chintankavathia deleted the feat/tooltip/configure-scroll-strategy branch May 7, 2026 16:02
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.

3 participants