ui: (fix) Tooltip component styling and props #46854
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
83 tasks
dhartunian
approved these changes
Apr 1, 2020
There was a problem hiding this comment.
Can you update the release notes as the tooltip visibility problem is a bug:
This fixes a bug for statement diagnostics that led to tooltips always showing instead of only on hover"
I don't think we need to call out the tooltip width since that doesn't affect content.
Reviewable status:
complete! 1 of 0 LGTMs obtained
This fix is rework of previous changes related to PR: cockroachdb#46557 Prior, to customize tooltip width for two particular cases, changes were made in shared component and affected all tooltips across project (tooltip width was set to 500px). Instead of this, current changes keep component styles without local changes and extend them with specific classes (via `overlayClassName` prop). It allows to apply changes in specific places (Statements and Jobs tables). Next fix: the order of destructing props in `components/tooltip/tooltip.tsx`. `{...props}` supplied as a last prop to `<AntTooltip />` component and it overrides all previous props which have to be preserved. To fix this, ...props was moved as first prop. And last fix: Tooltips for Diagnostics Status Badge was set to be visible always and with some random conditions tooltips appeared and were displayed instantly. To fix this, `visible` prop was removed to trigger tooltip visibility only on mouse hover. And to position Diagnostics Status Badge tooltip more elegantly - it is positioned to `bottomLeft` side, because this badge is displayed in the last columns and there is not enough place on the right side for tooltip. Release note (bug fix): Tooltips for statement diagnostics were shown always instead of only on hover Release justification: bug fixes and low-risk updates to new functionality
koorosh
force-pushed
the
ui-fix-tooltip-styling
branch
from
bedd398 to
a2dde8d
Compare
Done! |
|
bors r+ |
Build succeeded |
This was referenced Apr 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix is rework of previous changes related to PR: #46557
Prior, to customize tooltip width for two particular cases,
changes were made in shared component and affected all tooltips
across project (tooltip width was set to 500px).
For example, tooltips for Diagnostics badges were 500px wide:
Instead of this, current changes keep component styles without
local changes and extend them with specific classes (via
overlayClassNameprop). It allows to apply changes in specific places (Statements and Jobs
tables).
Next fix: the order of destructing props in
components/tooltip/tooltip.tsx.{...props}supplied as a last prop to<AntTooltip />component and itoverrides all previous props which have to be preserved. To fix this, ...props
was moved as first prop.
And last fix: Tooltips for Diagnostics Status Badge was set to be visible always
and with some random conditions tooltips appeared and were displayed instantly.
To fix this,
visibleprop was removed to trigger tooltip visibility only onmouse hover.
And to position Diagnostics Status Badge tooltip more elegantly - it is positioned
to
bottomLeftside, because this badge is displayed in the last columns and thereis not enough place on the right side for tooltip.
Release note (bug fix): Tooltips for statement diagnostics were shown always
instead of only on hover
Release justification: bug fixes and low-risk updates to new functionality