-
Notifications
You must be signed in to change notification settings - Fork 159
fix(overlay): conditionally cache element size - master #16837
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses issue #16458 by adding conditional caching of element sizes in the overlay service. The fix introduces a new cacheSize setting that controls whether an element's dimensions should be captured before or after moving it into the overlay container.
Changes:
- Added
cacheSizeboolean flag toOverlaySettingsinterface - Modified overlay element size calculation logic to conditionally cache dimensions based on the new flag
- Set
cacheSize: falsefor tooltips and notifications to prevent size inheritance issues
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
utilities.ts |
Added cacheSize property to OverlaySettings interface |
overlay.ts |
Implemented conditional size caching logic and set default cacheSize: true |
tooltip-target.directive.ts |
Disabled size caching for tooltips |
notifications.directive.ts |
Disabled size caching for notifications |
overlay.spec.ts |
Updated tests to verify conditional size caching behavior |
projects/igniteui-angular/core/src/services/overlay/utilities.ts
Outdated
Show resolved
Hide resolved
wnvko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we know this PR is breaking the tooltip. What is the reason this happening? Can we revert the change that made this happen?
Adding this additional property to the overlay settings and use it in this way in the tooltip looks like a breaking change. IMO it will be better to find out what happened in the above mentioned PR and to revert it.
Closes #16458
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)