-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(material/tooltip): avoid problem when triggers hide animation for not visible tooltip #24652
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
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). For more information, open the CLA check for this pull request. |
194f3dd
to
5746c5d
Compare
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.
A couple of notes:
- I think that the correct approach would be to clear the tooltip opening timeout if the pointer is moved away.
- I noticed this when I was reworking the tooltip in a5ab8e9, but I ended up keeping the old behavior, because it caused a lot of screenshot failures internally.
The problem is that during the After mouseleave event open timeout wipes. |
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.
LGTM
Any update on this? |
Hello @volvachev, we're running this PR through Google's internal presubmit. Could you please rebase this branch with master? |
5746c5d
to
9c7ccef
Compare
Hi, I did it. |
… not visible tooltip Fixes a bug in Angular Material `tooltip` when `mat-tooltip-hide` class trigger animation for not visible tooltip. Fixes angular#24614
9c7ccef
to
f62210d
Compare
tyty, the most recent presubmit process from earlier this week had less than ten failures. I'm kicking off a fresh one. |
So, will my fix be included in new one? |
Any updates? |
@volvachev The presubmit process Zach was describing was for this PR. Each PR is processed independently From the latest presubmit run, I can see that it only had a handful of failures so it (hopefully) shouldn't be too difficult to get through. Unfortunately I haven't been able to prioritize this enough to sit down and debug each failure |
Hello! It looks like there are no failed checks anymore. Is there any update on this PR? |
Any news about this PR? The impacts of this bug are quite unpleasant for the users. |
are we there yet? |
Pretty please? 🙏 |
I'll take another look. We got this down pretty close but the internal failures are tough to debug. After rerunning internal tests tonight I'll post an update on how things are looking |
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.
lgtm
…tion for not visible tooltip (angular#24652)" This reverts commit 3041cda.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`14.1.1` -> `14.1.2`](https://renovatebot.com/diffs/npm/@angular%2fcdk/14.1.1/14.1.2) | | [@angular/material](https://github.com/angular/components) | dependencies | patch | [`14.1.1` -> `14.1.2`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/14.1.1/14.1.2) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v14.1.2`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1412-cedar-cicada-2022-08-11) [Compare Source](angular/components@14.1.1...14.1.2) ##### material | Commit | Type | Description | | -- | -- | -- | | [69b29046d5](angular/components@69b2904) | fix | **chips:** `selectable="false"` doesn't work on initial assignment ([#​24906](angular/components#24906)) | | [0f2ec70c86](angular/components@0f2ec70) | fix | **tooltip:** avoid problem when triggers hide animation for not visible tooltip ([#​24652](angular/components#24652)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [0a4d8e9fc4](angular/components@0a4d8e9) | fix | **mdc-list:** set a role on MatNavList and MatActionList ([#​25412](angular/components#25412)) | #### Special Thanks Egor Volvachev, Miles Malerba, Wagner Maciel and Zach Arend <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTQuNyIsInVwZGF0ZWRJblZlciI6IjMyLjE1NC43In0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1503 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR fixes the problem when mat-tooltip-hide triggers the animation of tooltip that has not been shown yet.
Fixes #24614
Caretaker note: Integration test is failing because selenium wants to click on a select item but instead is clicking on the tooltip