chore: Shows the dataset description in the gallery dropdown #16200

michael-s-molina · 2021-08-11T18:01:13Z

SUMMARY

Shows the dataset description in the gallery dropdown
Lazy load the data source values with search and pagination

Closes #16153

@junlincc @rusackas @jinghua-qa

@villebro After this PR is merged we need to remove the datasets from bootstrap data.

AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-08-12.at.4.21.38.PM.mov

TESTING INSTRUCTIONS

Check the original issue for instructions.

ADDITIONAL INFORMATION

Has associated issue:
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-08-11T20:24:19Z

Codecov Report

Merging #16200 (7accf42) into master (ccfc95f) will decrease coverage by 0.20%.
The diff coverage is 74.40%.

❗ Current head 7accf42 differs from pull request most recent head 6d4f06c. Consider uploading reports for the commit 6d4f06c to get more accurate results

@@            Coverage Diff             @@
##           master   #16200      +/-   ##
==========================================
- Coverage   76.73%   76.53%   -0.21%     
==========================================
  Files         996      997       +1     
  Lines       52999    53194     +195     
  Branches     6738     6764      +26     
==========================================
+ Hits        40668    40711      +43     
- Misses      12102    12253     +151     
- Partials      229      230       +1

Flag	Coverage Δ
hive	`?`
mysql	`81.54% <81.38%> (-0.07%)`	⬇️
postgres	`81.60% <81.38%> (-0.03%)`	⬇️
presto	`?`
python	`81.69% <81.38%> (-0.46%)`	⬇️
sqlite	`81.21% <81.38%> (-0.07%)`	⬇️

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

Impacted Files	Coverage Δ
superset-frontend/src/addSlice/App.tsx	`0.00% <ø> (ø)`
superset-frontend/src/components/Form/Form.tsx	`100.00% <ø> (ø)`
...ontrols/DndColumnSelectControl/DndColumnSelect.tsx	`46.42% <ø> (-0.94%)`	⬇️
...rontend/src/explore/components/optionRenderers.tsx	`100.00% <ø> (ø)`
...perset-frontend/src/views/CRUD/chart/ChartList.tsx	`72.61% <ø> (ø)`
...set-frontend/src/views/CRUD/data/database/types.ts	`100.00% <ø> (ø)`
...set-frontend/src/views/CRUD/welcome/ChartTable.tsx	`73.17% <0.00%> (-1.83%)`	⬇️
...c/visualizations/TimeTable/TimeTableChartPlugin.js	`75.00% <ø> (ø)`
superset/common/query_object.py	`90.66% <ø> (ø)`
superset/datasets/api.py	`91.92% <ø> (ø)`
... and 57 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 ccfc95f...6d4f06c. Read the comment docs.

junlincc · 2021-08-12T01:40:46Z

/testenv up

github-actions · 2021-08-12T01:42:48Z

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

junlincc · 2021-08-12T02:41:23Z

tested locally, LGTM
also tried remove, edit the description, it's reflected accurately in the tooltip

junlincc

product sign-off ✅

mistercrunch · 2021-08-12T04:39:12Z

Can we make it an info bubble? That way we see clearly which datasets have a description and which don't, and tooltips don't get in the way if you don't want to see them.

Something like what we do in the metadata panel?

Also, could be another PR but would love to see this surfaced in a similar way in CRUD list view!

michael-s-molina · 2021-08-12T14:10:17Z

Can we make it an info bubble? That way we see clearly which datasets have a description and which don't, and tooltips don't get in the way if you don't want to see them.

Something like what we do in the metadata panel?

Also, could be another PR but would love to see this surfaced in a similar way in CRUD list view!

@mistercrunch Currently, the tooltip is always displayed by Ant Design and it's really helpful for cases where the text is truncated. That said, we did some changes in other places to only show the tooltip if the text is truncated and I liked the idea. I agree that having an icon is very helpful in this scenario because we have extra information, that may or may not be present for a dataset. I'll do the requested changes in a follow-up PR because we need to change the Select component and deal with the combination of truncated text, not truncated text, and icon.

kgabryje · 2021-08-12T15:13:37Z

When I open the datasets select for the first time, I see a "No data" placeholder for about a second. Can we not show it? Or replace with some spinner?

Also, when datasets description is very long (like World Bank Health Data from test datasets) has a very long description that doesn't fit in the tooltip. Can we show 3 dots at the end or something?

kgabryje · 2021-08-12T15:14:34Z

superset-frontend/src/addSlice/AddSliceContainer.tsx

+        }) => ({
+          value: `${item.id}__${item.datasource_type}`,
+          label: item.table_name,
+          title: `${item.table_name}\n\n${


Can we replace \n\n with a span with some padding, so that we have more control over the spacings and can use grid unit from theme?

geido

The implementation looks good. However, I also share some concerns about these tooltips.

Why aren't we using the Antd Tooltip as everywhere else? I feel like this is harder to read.

I think we shouldn't be showing "No description" where there is no description available.

michael-s-molina · 2021-08-12T16:15:39Z

@kgabryje @geido Thanks for the comments!

When I open the datasets select for the first time, I see a "No data" placeholder for about a second. Can we not show it? Or replace with some spinner?

This is the default behavior of the Select component. We can try to change that but I would like to only change the Select's code after we write the tests for the component to avoid regressions at this point. It's a little tricky because of the sync and async modes. I'll add your suggestion to the component's list of improvements.

Also, when datasets description is very long (like World Bank Health Data from test datasets) has a very long description that doesn't fit in the tooltip. Can we show 3 dots at the end or something?

Yes. Will do.

Can we replace \n\n with a span with some padding, so that we have more control over the spacings and can use grid unit from theme?

Yes. Will do.

Why aren't we using the Antd Tooltip as everywhere else? I feel like this is harder to read.

I was using the native feature of the Ant Design Select. To use our tooltip we need to change the component. I'll add your suggestion to the component's list of improvements.

I think we shouldn't be showing "No description" where there is no description available.

The idea here was to show the user that he has the possibility to add a description. We'll add the info bubble in a follow-up PR according to @mistercrunch suggestions. I'm imagining that the icon will only appear for items that have a description so your concern will disappear.

michael-s-molina · 2021-08-12T19:34:32Z

@kgabryje @geido I was able to change the tooltip without modifying the Select component! With that, all your requests with the exception of the 'No data' were implemented.

I updated the PR's description with the latest video.

This PR is the base for another important PR so let's treat the following improvements in follow-up PRs:

Show icons for datasets that contain descriptions @mistercrunch
Apply the same logic to the CRUD list view @mistercrunch
Only show tooltips when necessary (truncated items or datasets with descriptions)

rusackas · 2021-08-12T19:53:40Z

/testenv up

github-actions · 2021-08-12T19:57:59Z

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

rusackas · 2021-08-12T20:03:32Z

superset-frontend/src/addSlice/AddSliceContainer.tsx

+    .tooltip-description {
+      margin-top: ${theme.gridUnit * 2}px;
+      display: -webkit-box;
+      -webkit-line-clamp: 20;


Ooh, you don't get to use that every day.

rusackas · 2021-08-12T20:10:18Z

Love the new tooltips, but it might be nice to position them so they hang off the right side of the Select option. Right now, the tooltip covers the other menu items, which means it gets in the way of selecting anything. Kind of frustrating.

rusackas · 2021-08-12T20:12:23Z

superset-frontend/src/addSlice/AddSliceContainer.tsx

+      <Tooltip
+        title={


Suggested change

<Tooltip

title={

<Tooltip

placement="right"

title={

villebro · 2021-08-13T07:26:14Z

I love the lazy loading of datasource data! As we now no longer need it in bootstrap data, we can pull it out with this change to make page loading marginally faster/lighter:

diff --git a/superset/views/chart/views.py b/superset/views/chart/views.py
index 68c19cc2c..01b9e53ca 100644
--- a/superset/views/chart/views.py
+++ b/superset/views/chart/views.py
@@ -62,15 +62,7 @@ class SliceModelView(
     @expose("/add", methods=["GET", "POST"])
     @has_access
     def add(self) -> FlaskResponse:
-        datasources = [
-            {"value": str(d.id) + "__" + d.type, "label": repr(d)}
-            for d in security_manager.get_user_datasources()
-        ]
         payload = {
-            "datasources": sorted(
-                datasources,
-                key=lambda d: d["label"].lower() if isinstance(d["label"], str) else "",
-            ),
             "common": common_bootstrap_payload(),
             "user": bootstrap_user_data(g.user),
         }

Other than that I love @rusackas propsal to place the tooltip on the right - LGTM after these changes.

geido

LGTM with the suggestions above!

michael-s-molina · 2021-08-13T16:09:55Z

@rusackas I was able to adjust the tooltip positioning. Now, I'm presenting the tooltip at the row level, always preserving the visibility of the items independently of the content length.

Screen.Recording.2021-08-13.at.1.03.53.PM.mov

@villebro I removed the bootstrap data according to your instructions. Thanks!

…unnecessary bootstrap data

rusackas · 2021-08-13T16:21:16Z

/testenv up

github-actions · 2021-08-13T16:25:53Z

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

github-actions · 2021-08-13T18:08:33Z

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

…16200) * chore: Shows the dataset description in the gallery dropdown * chore: Adjusts the tooltip positioning, fixes the search and removes unnecessary bootstrap data

michael-s-molina requested review from rusackas, pkdotson and villebro August 11, 2021 18:01

superset-github-bot bot added the Turing label Aug 11, 2021

pull-request-size bot added the size/M label Aug 11, 2021

michael-s-molina force-pushed the viz-gallery-dataset-description branch from a14008f to 81fd89e Compare August 11, 2021 19:58

junlincc added the viz:gallery label Aug 12, 2021

junlincc reviewed Aug 12, 2021

View reviewed changes

michael-s-molina force-pushed the viz-gallery-dataset-description branch 2 times, most recently from f37b0c0 to 71e9aae Compare August 12, 2021 12:20

michael-s-molina requested a review from geido August 12, 2021 14:11

kgabryje reviewed Aug 12, 2021

View reviewed changes

geido reviewed Aug 12, 2021

View reviewed changes

chore: Shows the dataset description in the gallery dropdown

88af84a

michael-s-molina force-pushed the viz-gallery-dataset-description branch from 71e9aae to 88af84a Compare August 12, 2021 19:23

pull-request-size bot added size/L and removed size/M labels Aug 12, 2021

rusackas reviewed Aug 12, 2021

View reviewed changes

geido approved these changes Aug 13, 2021

View reviewed changes

chore: Adjusts the tooltip positioning, fixes the search and removes …

6d4f06c

…unnecessary bootstrap data

michael-s-molina force-pushed the viz-gallery-dataset-description branch from e4f4a6f to 6d4f06c Compare August 13, 2021 16:12

michael-s-molina requested a review from rusackas August 13, 2021 16:13

michael-s-molina merged commit 720e5b1 into apache:master Aug 13, 2021

This was referenced Aug 13, 2021

chore: Improves the flow to create a new chart #16252

Merged

feat(explore): make dnd controls clickable #16119

Merged

etr2460 mentioned this pull request Aug 24, 2021

feat: Improve sorting for new chart dataset select #16414

Closed

9 tasks

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

chore: Shows the dataset description in the gallery dropdown #16200

chore: Shows the dataset description in the gallery dropdown #16200

michael-s-molina commented Aug 11, 2021 •

edited

Loading

codecov bot commented Aug 11, 2021 •

edited

Loading

junlincc commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

junlincc commented Aug 12, 2021

junlincc left a comment

mistercrunch commented Aug 12, 2021 •

edited

Loading

michael-s-molina commented Aug 12, 2021 •

edited

Loading

kgabryje commented Aug 12, 2021

kgabryje Aug 12, 2021

geido left a comment

michael-s-molina commented Aug 12, 2021

michael-s-molina commented Aug 12, 2021 •

edited

Loading

rusackas commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

rusackas Aug 12, 2021

rusackas commented Aug 12, 2021

rusackas Aug 12, 2021

villebro commented Aug 13, 2021

geido left a comment

michael-s-molina commented Aug 13, 2021

rusackas commented Aug 13, 2021

github-actions bot commented Aug 13, 2021

github-actions bot commented Aug 13, 2021

chore: Shows the dataset description in the gallery dropdown #16200

chore: Shows the dataset description in the gallery dropdown #16200

Conversation

michael-s-molina commented Aug 11, 2021 • edited Loading

SUMMARY

AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Aug 11, 2021 • edited Loading

Codecov Report

junlincc commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

junlincc commented Aug 12, 2021

junlincc left a comment

Choose a reason for hiding this comment

mistercrunch commented Aug 12, 2021 • edited Loading

michael-s-molina commented Aug 12, 2021 • edited Loading

kgabryje commented Aug 12, 2021

kgabryje Aug 12, 2021

Choose a reason for hiding this comment

geido left a comment

Choose a reason for hiding this comment

michael-s-molina commented Aug 12, 2021

michael-s-molina commented Aug 12, 2021 • edited Loading

rusackas commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

rusackas Aug 12, 2021

Choose a reason for hiding this comment

rusackas commented Aug 12, 2021

rusackas Aug 12, 2021

Choose a reason for hiding this comment

villebro commented Aug 13, 2021

geido left a comment

Choose a reason for hiding this comment

michael-s-molina commented Aug 13, 2021

rusackas commented Aug 13, 2021

github-actions bot commented Aug 13, 2021

github-actions bot commented Aug 13, 2021

michael-s-molina commented Aug 11, 2021 •

edited

Loading

codecov bot commented Aug 11, 2021 •

edited

Loading

mistercrunch commented Aug 12, 2021 •

edited

Loading

michael-s-molina commented Aug 12, 2021 •

edited

Loading

michael-s-molina commented Aug 12, 2021 •

edited

Loading