Skip to content
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

fix(ui): Revert change to disabled button tooltips #29085

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

scttcper
Copy link
Member

@scttcper scttcper commented Oct 5, 2021

Quickly mousing over a disabled button with a tooltip would cause the tooltip to persist forever

@scttcper scttcper requested review from priscilawebdev and a team October 5, 2021 20:41
@scttcper scttcper requested a review from a team as a code owner October 5, 2021 20:41
@evanpurkhiser
Copy link
Member

@priscilawebdev what was the change for specifically?

@priscilawebdev
Copy link
Member

priscilawebdev commented Oct 6, 2021

the problem was that when the button was disabled in the dynamic sampling modal, no tooltip was being shown. The change solved the issue

image

@priscilawebdev
Copy link
Member

priscilawebdev commented Oct 6, 2021

Please where is this problem happening? it seems to work fine for me. if we merge this change the tooltip will no longer be displayed on the disabled button

disabled-button

@scttcper
Copy link
Member Author

scttcper commented Oct 6, 2021

@priscilawebdev here's where i saw it

Kapture.2021-10-06.at.09.59.17.mp4

Quickly mousing over a disabled button with a tooltip would cause the tooltip to persist forever
@scttcper
Copy link
Member Author

scttcper commented Oct 6, 2021

@priscilawebdev maybe we can enable overriding the skipWrapper for that use case or use a tooltip to wrap that button directly

@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants