Skip to content

ui: (fix) Tooltip component styling and props#46854

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
koorosh:ui-fix-tooltip-styling
Apr 1, 2020
Merged

ui: (fix) Tooltip component styling and props#46854
craig[bot] merged 1 commit intocockroachdb:masterfrom
koorosh:ui-fix-tooltip-styling

Conversation

@koorosh
Copy link
Copy Markdown
Contributor

@koorosh koorosh commented Apr 1, 2020

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:

Screenshot 2020-04-01 at 17 58 55

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).

Screenshot 2020-04-01 at 17 52 20

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.

Screenshot 2020-04-01 at 17 51 35

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 koorosh requested a review from a team April 1, 2020 14:56
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@jordanlewis jordanlewis mentioned this pull request Apr 1, 2020
83 tasks
Copy link
Copy Markdown
Contributor

@nathanstilwell nathanstilwell left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

:lgtm: Great work @koorosh!

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: :shipit: 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 koorosh force-pushed the ui-fix-tooltip-styling branch from bedd398 to a2dde8d Compare April 1, 2020 17:58
@koorosh
Copy link
Copy Markdown
Contributor Author

koorosh commented Apr 1, 2020

Can you update the release notes as the tooltip visibility problem is a bug

Done!

@koorosh koorosh requested a review from dhartunian April 1, 2020 17:59
@dhartunian
Copy link
Copy Markdown
Collaborator

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Apr 1, 2020

Build succeeded

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.

5 participants