fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel #16060

geido · 2021-08-04T13:49:20Z

SUMMARY

This PR makes the tooltip appear only when the label does not fit the container for the METRICS/FILTERS/GROUP BY (with D&D enabled)/SORT BY of the DATA panel. It also consolidates the look and behavior of the Columns and Metrics across the 3 different implementations, including the one in Superset UI. Finally, this PR hides the tooltip when the drag event starts.

REQUIRES SUPERSET-UI PR MERGED

Attention, this PR requires the related PR apache-superset/superset-ui#1271 in Superset-UI to be merged first.

REFACTOR REQUIRED

Refactoring is suggested to remove the 3 different implementations and consolidate the code. This PR only intention is to resolve the issue #15996 within the limitations of the current implementation.

BEFORE

before_drag_drop.mp4

before_NO_drag_drop.mp4

AFTER

after_drag_drop.mp4

after_NO_drag_drop.mp4

TESTING INSTRUCTIONS

Visit Explore
Add Columns and Metrics in Filters and Sort options either with drag and drop enabled or not
Observe the labels with long text and make sure they are truncated with ellipsis
Hover the labels and check the tooltip
Expand the panel until the text isn't truncated and hover the labels
Make sure the tooltip isn't appearing
Drag an item
Make sure the tooltip isn't appearing

ADDITIONAL INFORMATION

Has associated issue: [d&d]Only show tooltip when full text doesn't fit + close tooltip as Drag starts #15996
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

geido · 2021-08-04T14:06:06Z

@rosemarie-chiu @jinghua-qa

junlincc · 2021-08-04T14:54:18Z

Insane!! you made it work!!

please spin up the testenv with DND enabled 🙏

geido · 2021-08-04T18:27:29Z

For clarity:

This PR only affects the METRICS/FILTERS/GROUP BY (with D&D enabled)/SORT BY of the DATA panel and nothing else. It does not change the behavior of the Metrics and Columns anywhere else, including the ones in the Dataset panel, for which an additional implementation will be required.
Specifically for the GROUP BY when D&D is off, the component is using the about-to-be-deprecated react-select, so it does not make sense to invest in it as it is not a blocker for the original issue and should be resolved when the Antdesign Select component will replace it.

Ideally, we should strive to implement the ability to show the tooltip only when it does not fit the container everywhere. However, this is an effort that will require more iterations.

geido · 2021-08-04T19:20:35Z

This PR needs to wait for #15942 to be merged as that PR contains the required Superset-UI version. The CI checks won't pass until that's merged in.

codecov · 2021-08-05T06:56:40Z

Codecov Report

Merging #16060 (2398c7b) into master (423ff50) will increase coverage by 0.00%.
The diff coverage is 88.63%.

@@           Coverage Diff           @@
##           master   #16060   +/-   ##
=======================================
  Coverage   76.83%   76.84%           
=======================================
  Files         995      995           
  Lines       52884    52920   +36     
  Branches     6721     6743   +22     
=======================================
+ Hits        40636    40667   +31     
- Misses      12023    12028    +5     
  Partials      225      225

Flag	Coverage Δ
javascript	`71.23% <88.63%> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
...ontrols/DndColumnSelectControl/DndColumnSelect.tsx	`45.45% <ø> (-0.98%)`	⬇️
...rontend/src/explore/components/optionRenderers.tsx	`100.00% <ø> (ø)`
.../controls/DndColumnSelectControl/OptionWrapper.tsx	`85.96% <87.50%> (+0.25%)`	⬆️
...plore/components/controls/OptionControls/index.tsx	`89.90% <89.47%> (-0.52%)`	⬇️
...ontrols/DndColumnSelectControl/DndFilterSelect.tsx	`45.07% <100.00%> (ø)`

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 423ff50...2398c7b. Read the comment docs.

villebro · 2021-08-05T08:30:56Z

/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_UX_BETA=true

github-actions · 2021-08-05T08:33:24Z

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

villebro

LGTM - this is a really nice functional improvement and code also looks much cleaner now 👍

superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx

superset-frontend/src/explore/components/controls/OptionControls/index.tsx

…ls/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

junlincc · 2021-08-05T20:14:52Z

@geido thanks for the fix! We also need to make the change of the data panel. tooltip should only show when it doesn't fit in the datapanel 🙏

junlincc · 2021-08-05T20:15:36Z

@jinghua-qa it appears that this can be reproduced on Safari only. However, this is not related to this PR and we would need to address it separetely on Safari on a general level.

Yes, i can confirm it;s not related to this PR. let's have a follow up one to clean up in Safari

But let's not close the issue until above 2 comments are addressed. 🙏

junlincc · 2021-08-05T20:31:04Z

Also in filter, when there are multiple value selected, should we? we should? show all the selected values in the tooltip, instead of "..." this change can be made in this PR, maybe? @geido

Screen.Recording.2021-08-05.at.10.29.31.AM.mov

junlincc

All selected filter values show show in the tooltip 🙏

villebro · 2021-08-06T06:41:42Z

We also need to make the change of the data panel. tooltip should only show when it doesn't fit in the datapanel 🙏

Agreed, this is a major pain! But I vote for catching that in a follow-up.

geido · 2021-08-06T09:30:02Z

Tooltip in Filters

I have just updated the PR to show the tooltip whenever the tooltip title is different from the label (i.e. the tooltip text is longer than what fits in the label in every size of the panel).

In that way, the tooltip will show up in cases such as the "Filters" IN and NOT IN and in any other case where we might want to force-show the tooltip independently from the label content or the size of the panel.

DEV.Quarterly.1.mp4

Tooltips in Dataset panel

One PR has been opened for this:

#16192

Disabled Default Tooltips on Safari

Two PRs have been opened for this:

#16145
apache-superset/superset-ui#1283

CC @junlincc @villebro @michael-s-molina @jinghua-qa

geido · 2021-08-09T15:46:09Z

/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_UX_BETA=true

github-actions · 2021-08-09T15:50:26Z

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

junlincc

Product sign-off!! tested locally. will test again when both this and #16192 go in.

rusackas · 2021-08-12T17:46:47Z

superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx

+      labelRef &&
+      labelRef.current &&
+      labelRef.current.scrollWidth > labelRef.current.clientWidth);


Suggested change

labelRef &&

labelRef.current &&

labelRef.current.scrollWidth > labelRef.current.clientWidth);

labelRef?.current?.scrollWidth > labelRef.current.clientWidth);

If you're feelin' fancy ¯\_(ツ)_/¯

github-actions · 2021-08-12T17:48:37Z

Ephemeral environment shutdown and build artifacts deleted.

…gies * upstream/master: (64 commits) check roles before fetching reports (#16260) chore: upgrade mypy and add type guards (#16227) fix: pivot columns with ints for name (#16259) chore(pylint): Bump Pylint to 2.9.6 (#16146) fix examples tab for dashboard (#16253) chore: bump superset-ui packages to 0.17.84 (#16251) chore: Shows the dataset description in the gallery dropdown (#16200) fix(Dashboard): Omnibar dropdown visibility and keyboard commands (#16168) chore: bump py version for integration test (#16213) fix: skip perms on query context update (#16250) refactor: external metadata fetch API (#16193) feat(dao): admin can remove self from object owners (#15149) fix(dashboard): cross filter chart highlight when filters badge icon clicked (#16233) fix: validate_parameters and query (#16241) fix: Remove Advanced Analytics tag for 2 charts (#16240) Revert "feat: Changing Dataset names (#16199)" (#16235) feat: Allow users to connect via legacy SQLA form (#16201) fix: remove encryption from db params (#16214) fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (#16060) Show/hide tooltips (#16192) ... # Conflicts: # superset/tasks/caching/cache_strategy.py

…iner in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (apache#16060) * Implement dynamic tooltip * Normalize and consolidate * Clean up * Refactor and clean up * Remove unnecessary var * Fix type import * Update superset-frontend/src/explore/components/controls/OptionControls/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Remove unnecessary styled span * Show full tooltip title * Force show tooltip * Force show tooltip D&D off Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> (cherry picked from commit a1e18ed)

…iner in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (apache#16060) * Implement dynamic tooltip * Normalize and consolidate * Clean up * Refactor and clean up * Remove unnecessary var * Fix type import * Update superset-frontend/src/explore/components/controls/OptionControls/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Remove unnecessary styled span * Show full tooltip title * Force show tooltip * Force show tooltip D&D off Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

geido added 3 commits August 2, 2021 19:12

Implement dynamic tooltip

e282ae3

Normalize and consolidate

3c10fe3

Clean up

8e44b1c

superset-github-bot bot added the Flexiana label Aug 4, 2021

pull-request-size bot added the size/L label Aug 4, 2021

geido mentioned this pull request Aug 4, 2021

chore: Show tooltip conditionally and pass ref down apache-superset/superset-ui#1271

Merged

Merge

b4e4516

geido marked this pull request as ready for review August 4, 2021 14:04

geido requested review from kgabryje, michael-s-molina, junlincc, rusackas and villebro August 4, 2021 14:05

junlincc added #bug:blocking! Blocking issues with high priority rush! Requires immediate attention labels Aug 4, 2021

geido added 3 commits August 4, 2021 17:12

Refactor and clean up

72755bc

Remove unnecessary var

c252701

Fix type import

937dc04

geido changed the title ~~fix(Explore): Metrics and Columns to show tooltip only when label does not fit the container~~ fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel Aug 4, 2021

Merge branch 'master' into fix/issue_15996

5fe3be5

villebro approved these changes Aug 5, 2021

View reviewed changes

michael-s-molina reviewed Aug 5, 2021

View reviewed changes

superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx Outdated Show resolved Hide resolved

superset-frontend/src/explore/components/controls/OptionControls/index.tsx Outdated Show resolved Hide resolved

Update superset-frontend/src/explore/components/controls/OptionContro…

0c61289

…ls/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

junlincc added hold:testing! On hold for testing and removed #bug:blocking! Blocking issues with high priority labels Aug 5, 2021

junlincc requested changes Aug 5, 2021

View reviewed changes

geido added 4 commits August 6, 2021 10:53

Show full tooltip title

93e04fd

Force show tooltip

d17b7ee

Merge

48fcdfa

Force show tooltip D&D off

2398c7b

geido requested review from junlincc, villebro and michael-s-molina August 6, 2021 09:33

geido mentioned this pull request Aug 12, 2021

fix(Explore): Show the tooltip only when label does not fit the container in the Dataset panel #16192

Merged

8 tasks

michael-s-molina approved these changes Aug 12, 2021

View reviewed changes

junlincc removed the hold:testing! On hold for testing label Aug 12, 2021

junlincc approved these changes Aug 12, 2021

View reviewed changes

rusackas reviewed Aug 12, 2021

View reviewed changes

rusackas merged commit a1e18ed into apache:master Aug 12, 2021

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel #16060

fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel #16060

geido commented Aug 4, 2021 •

edited

Loading

geido commented Aug 4, 2021

junlincc commented Aug 4, 2021

geido commented Aug 4, 2021 •

edited

Loading

geido commented Aug 4, 2021 •

edited

Loading

codecov bot commented Aug 5, 2021 •

edited

Loading

villebro commented Aug 5, 2021

github-actions bot commented Aug 5, 2021

villebro left a comment

junlincc commented Aug 5, 2021

junlincc commented Aug 5, 2021 •

edited

Loading

junlincc commented Aug 5, 2021

junlincc left a comment

villebro commented Aug 6, 2021

geido commented Aug 6, 2021 •

edited

Loading

geido commented Aug 9, 2021

github-actions bot commented Aug 9, 2021

junlincc left a comment

rusackas Aug 12, 2021

github-actions bot commented Aug 12, 2021

fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel #16060

fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel #16060

Conversation

geido commented Aug 4, 2021 • edited Loading

SUMMARY

REQUIRES SUPERSET-UI PR MERGED

REFACTOR REQUIRED

BEFORE

AFTER

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

geido commented Aug 4, 2021

junlincc commented Aug 4, 2021

geido commented Aug 4, 2021 • edited Loading

geido commented Aug 4, 2021 • edited Loading

codecov bot commented Aug 5, 2021 • edited Loading

Codecov Report

villebro commented Aug 5, 2021

github-actions bot commented Aug 5, 2021

villebro left a comment

Choose a reason for hiding this comment

junlincc commented Aug 5, 2021

junlincc commented Aug 5, 2021 • edited Loading

junlincc commented Aug 5, 2021

junlincc left a comment

Choose a reason for hiding this comment

villebro commented Aug 6, 2021

geido commented Aug 6, 2021 • edited Loading

Tooltip in Filters

Tooltips in Dataset panel

Disabled Default Tooltips on Safari

geido commented Aug 9, 2021

github-actions bot commented Aug 9, 2021

junlincc left a comment

Choose a reason for hiding this comment

rusackas Aug 12, 2021

Choose a reason for hiding this comment

github-actions bot commented Aug 12, 2021

geido commented Aug 4, 2021 •

edited

Loading

geido commented Aug 4, 2021 •

edited

Loading

geido commented Aug 4, 2021 •

edited

Loading

codecov bot commented Aug 5, 2021 •

edited

Loading

junlincc commented Aug 5, 2021 •

edited

Loading

geido commented Aug 6, 2021 •

edited

Loading