fix(Explore): fixes broken layout of tooltips #14529

rusackas · 2021-05-07T18:39:26Z

SUMMARY

Fixes an issue where Explore control panels with long label text cause funky line-wrapping on mouseover. This was due to the span containing the tooltip(s) appearing inline, and getting to wide. This PR makes the tooltips absolutely positioned, so that they can't cause a line wrap. They might hang a little too far over to the right, but it's an improvement.

As bycatch, this moves a bunch of CSS from LESS files to the component itself, using Emotion and theme variables.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Screen.Recording.2021-05-19.at.9.43.19.PM.mov

After:

TEST PLAN

ADDITIONAL INFORMATION

Has associated issue: Fixes [explore] Dropdowns tooltip icon jumping when mouse is on label #12689
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2021-05-07T18:43:25Z

Codecov Report

Merging #14529 (2ad7a47) into master (1df9384) will increase coverage by 0.19%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master   #14529      +/-   ##
==========================================
+ Coverage   77.15%   77.35%   +0.19%     
==========================================
  Files         958      960       +2     
  Lines       48241    49357    +1116     
  Branches     5636     6055     +419     
==========================================
+ Hits        37219    38178     +959     
- Misses      10821    10959     +138     
- Partials      201      220      +19

Flag	Coverage Δ
hive	`80.80% <ø> (ø)`
javascript	`72.63% <77.77%> (+0.62%)`	⬆️
mysql	`81.06% <ø> (ø)`
postgres	`81.09% <ø> (ø)`
presto	`80.79% <ø> (-0.01%)`	⬇️
python	`81.62% <ø> (-0.01%)`	⬇️
sqlite	`80.70% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../src/explore/components/ControlPanelsContainer.tsx	`84.92% <75.00%> (-0.04%)`	⬇️
...-frontend/src/explore/components/ControlHeader.jsx	`85.71% <100.00%> (+0.52%)`	⬆️
superset-frontend/src/components/Menu/Menu.tsx	`64.86% <0.00%> (-4.61%)`	⬇️
superset-frontend/src/CRUD/Field.jsx	`79.41% <0.00%> (-3.93%)`	⬇️
...erset-frontend/src/SqlLab/components/SaveQuery.tsx	`71.83% <0.00%> (-3.17%)`	⬇️
superset-frontend/src/components/Menu/SubMenu.tsx	`96.93% <0.00%> (-3.07%)`	⬇️
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx	`68.77% <0.00%> (-1.46%)`	⬇️
...d/src/explore/components/PropertiesModal/index.tsx	`81.73% <0.00%> (-0.47%)`	⬇️
superset/db_engine_specs/presto.py	`89.89% <0.00%> (-0.43%)`	⬇️
superset-frontend/src/dashboard/actions/hydrate.js	`3.19% <0.00%> (-0.12%)`	⬇️
... and 50 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1df9384...2ad7a47. Read the comment docs.

villebro

I'm still getting the wrapping problem:

villebro · 2021-05-10T05:28:36Z

superset-frontend/src/explore/components/ControlPanelsContainer.tsx

+          .control-label {
+            margin-bottom: 0px;
+            position: relative;
+            span + span {


I was wondering if we should add class names to these and target those?

I went ahead and moved these styles to a better location. I'll sleep better now, thanks for the suggestion.

I think they're a little more solid now too... see if you can break it 🤞

rusackas · 2021-05-12T14:28:26Z

I'm still getting the wrapping problem:

Ugh... it fixes SOME of them. Some are coming from a different component, and will require additional styles. More commits landing here soon.

rusackas · 2021-05-13T06:24:22Z

@villebro You uncovered something gross... this PR only fixes SOME of the controls.

A bunch of controls use ControlHeader from superset, and some use ControlHeader from superset-ui 🤦

Proposed approach:

Merge this (it fixes several)
Merge a PR on superset-ui (which I'll open soon) that does the same thing with the ControlHeader on that side
Bump in the @superset-ui/chart-controls package, and use that updated ControlHeader from superset-ui everywhere, and delete the duplicate one (this one!) from Superset.

rusackas · 2021-05-13T06:29:17Z

Step 2 of the plan opened here: apache-superset/superset-ui#1107

michael-s-molina · 2021-05-14T18:20:32Z

superset-frontend/src/explore/components/ControlHeader.jsx

@@ -49,7 +49,16 @@ export default class ControlHeader extends React.Component {
  renderOptionalIcons() {
    if (this.props.hovered) {
      return (
-        <span>
+        <span
+          css={theme => css`


css 👀 ... 🤣

The debate of the ages! I'm actually working on a doc to specify use cases for each, and try to arrive at some sort of best practices. I'll share it when it's fully baked.

suddjian

LGTM in light of the plan™

junlincc · 2021-05-18T22:45:14Z

/testenv up

github-actions · 2021-05-19T03:00:24Z

@junlincc Ephemeral environment spinning up at http://34.214.11.39:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions · 2021-05-19T04:56:43Z

Ephemeral environment shutdown and build artifacts deleted.

* fix: moves LESS styles into Emotion, fixes broken layout of tooltips * fix: tests * style: linting * style: better styling location

fix: moves LESS styles into Emotion, fixes broken layout of tooltips

91b8eed

superset-github-bot bot added the preset-io label May 7, 2021

pull-request-size bot added the size/M label May 7, 2021

rusackas added 2 commits May 7, 2021 12:55

fix: tests

ebd78d7

style: linting

dead6f5

rusackas requested review from suddjian, nytai, kgabryje and villebro May 7, 2021 23:11

villebro reviewed May 10, 2021

View reviewed changes

rusackas added the bash! label May 12, 2021

junlincc added need:review Requires a code review hold! On hold and removed need:review Requires a code review labels May 13, 2021

style: better styling location

2ad7a47

rusackas mentioned this pull request May 13, 2021

fix: adjusts ControlHeader tooltip layout, exports ControlHeader apache-superset/superset-ui#1107

Closed

rusackas requested a review from villebro May 14, 2021 06:10

michael-s-molina reviewed May 14, 2021

View reviewed changes

suddjian approved these changes May 14, 2021

View reviewed changes

junlincc changed the title ~~fix: moves LESS styles into Emotion, fixes broken layout of tooltips~~ fix(Explore): fixes broken layout of tooltips May 18, 2021

junlincc removed the hold! On hold label May 18, 2021

rusackas merged commit c2bd684 into master May 19, 2021

rusackas deleted the fix-control-info-tooltip-positioning branch May 19, 2021 04:55

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Explore): fixes broken layout of tooltips #14529

fix(Explore): fixes broken layout of tooltips #14529

rusackas commented May 7, 2021 •

edited by junlincc

codecov bot commented May 7, 2021 •

edited

villebro left a comment

villebro May 10, 2021

rusackas May 13, 2021

rusackas May 13, 2021

rusackas commented May 12, 2021

rusackas commented May 13, 2021

rusackas commented May 13, 2021

michael-s-molina May 14, 2021

rusackas May 18, 2021

suddjian left a comment •

edited

junlincc commented May 18, 2021

github-actions bot commented May 19, 2021

github-actions bot commented May 19, 2021

fix(Explore): fixes broken layout of tooltips #14529

fix(Explore): fixes broken layout of tooltips #14529

Conversation

rusackas commented May 7, 2021 • edited by junlincc

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov bot commented May 7, 2021 • edited

Codecov Report

villebro left a comment

Choose a reason for hiding this comment

villebro May 10, 2021

Choose a reason for hiding this comment

rusackas May 13, 2021

Choose a reason for hiding this comment

rusackas May 13, 2021

Choose a reason for hiding this comment

rusackas commented May 12, 2021

rusackas commented May 13, 2021

rusackas commented May 13, 2021

michael-s-molina May 14, 2021

Choose a reason for hiding this comment

rusackas May 18, 2021

Choose a reason for hiding this comment

suddjian left a comment • edited

Choose a reason for hiding this comment

junlincc commented May 18, 2021

github-actions bot commented May 19, 2021

github-actions bot commented May 19, 2021

rusackas commented May 7, 2021 •

edited by junlincc

codecov bot commented May 7, 2021 •

edited

suddjian left a comment •

edited