chore: implement new mockup to the new viz gallery (2nd iteration) #15868

stephenLYZ · 2021-07-23T16:31:34Z

SUMMARY

This PR is the second iteration of viz gallery.

To relocate the search bar to main section
Search to take up the entire row
Stack tags above category list, only display Highly-used, ECharts, Advanced-Analytics
Click to expand and show 4 sections:
- recommended tags
- all
- category
- tags
from global new button - anchor to show recommended tags (implemented)
from explore(switching viz) -anchor to show existing chart and its category (implemented)
open from dataset list -> explore -> anchor to show recommended tags (not implemented)

related: #15734
design: https://www.figma.com/file/LMnulBPitqSh5GeElVbsUV/chart-popover?node-id=0%3A1

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

show existing chart and scroll to the category, enable the user to see the selected category.

2021-07-24.12.25.59.mov

TESTING 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

suddjian · 2021-07-23T17:09:04Z

superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx

-`;
-
-const CategoriesWrapper = styled.div`
+const Wrapper = styled.div`


Wrapper isn't a very descriptive name. I was going to suggest renaming it so that it indicates what it is wrapping, but then I noticed that it lives directly within the LeftPane. Is it possible to merge these styles into LeftPane and remove the Wrapper component?

I think it's possible, I'll try it.

suddjian · 2021-07-23T17:21:55Z

Suggestion, non-blocking:
Is Advanced-Analytics the longest tag we're dealing with? It looks like it should be able to fit within the left pane. Perhaps we can have the left pane size itself to fit its contents to avoid the need for a popover.

suddjian · 2021-07-23T17:23:55Z

/testenv up

github-actions · 2021-07-23T17:26:00Z

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

stephenLYZ · 2021-07-23T17:30:35Z

Suggestion, non-blocking:
Is Advanced-Analytics the longest tag we're dealing with? It looks like it should be able to fit within the sidebar. Perhaps we can have the sidebar size itself to fit its contents to avoid the need for a popover.

Good idea! I agree that we do not limit the sidebar size, and we also don't need tooltip.

suddjian · 2021-07-23T17:55:39Z

superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx

+              }
+              key={COLLAPSE_LABEL.ALL_CHARTS}
+            >
+              <Selector


It's a bit strange to have a collapse panel that will only ever have one item in it. Should we put the "All Charts" selector at the top of the left pane instead, outside of the collapse?

suddjian · 2021-07-23T18:28:29Z

superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx

+            ghost
+            defaultActiveKey={Object.values(COLLAPSE_LABEL)}
+          >
+            <Collapse.Panel


Might be able to reduce some duplication by making an array of selector groups and maping that to render a collapse panel for each group.

Or actually I think a better way to reduce duplication would be adding a component <SelectorGroup> that renders a Collapse Panel and its contents.

suddjian · 2021-07-23T22:04:48Z

Played with this in the ephemeral env, looks good! Awesome work.

junlincc · 2021-07-23T23:34:14Z

@suddjian thanks for the review!!

codecov · 2021-07-24T16:16:05Z

Codecov Report

Merging #15868 (fe8120f) into master (d408ff8) will increase coverage by 0.01%.
The diff coverage is 82.66%.

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

@@            Coverage Diff             @@
##           master   #15868      +/-   ##
==========================================
+ Coverage   77.08%   77.09%   +0.01%     
==========================================
  Files         984      984              
  Lines       51787    51819      +32     
  Branches     7031     7043      +12     
==========================================
+ Hits        39918    39949      +31     
  Misses      11644    11644              
- Partials      225      226       +1

Flag	Coverage Δ
javascript	`71.75% <85.91%> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
superset-frontend/src/components/Icons/index.tsx	`100.00% <ø> (ø)`
...frontend/src/views/CRUD/alert/AlertReportModal.tsx	`61.70% <ø> (ø)`
superset/exceptions.py	`93.18% <ø> (ø)`
superset/views/base.py	`75.86% <ø> (ø)`
superset/views/core.py	`75.21% <0.00%> (+0.16%)`	⬆️
...c/components/ErrorMessage/DatabaseErrorMessage.tsx	`90.90% <83.33%> (-3.83%)`	⬇️
...ponents/controls/VizTypeControl/VizTypeGallery.tsx	`93.65% <86.15%> (-0.62%)`	⬇️
superset/config.py	`91.24% <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 d408ff8...6a49026. Read the comment docs.

github-actions · 2021-07-25T11:10:54Z

@stephenLYZ Ephemeral environment creation is currently limited to committers.

suddjian · 2021-07-26T16:41:38Z

/testenv up

github-actions · 2021-07-26T16:45:42Z

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

kgabryje · 2021-07-26T17:13:03Z

Looks nice! 1 nit which is probably not related to this PR - there's a short vertical scroll in some charts' descriptions which looks a bit weird. I wonder if we could get rid of it somehow (shorter descriptions? fewer tags? larger modal?)

Screen.Recording.2021-07-26.at.19.09.58.mov

cc @junlincc

suddjian · 2021-07-26T17:31:51Z

I would like to see the "All Charts" section moved to the top and outside of a Collapse. Thoughts @junlincc ?

kgabryje · 2021-07-26T17:39:55Z

1 more thing - we have an "X" button on "All charts" label, which doesn't do anything. Maybe "All charts" shouldn't have that button?

junlincc · 2021-07-26T17:41:16Z

👍Let's make above changes in a follow up PR>

junlincc

Product sign-off ❣️great jobs everyone!! thank you so much! 🙇‍♀️

suddjian

code looks good, nice job

github-actions · 2021-07-26T18:11:10Z

Ephemeral environment shutdown and build artifacts deleted.

…pache#15868) * chore: implement new mockup to the new viz gallery * fix: update package-lock * fix: add license * fix: reduce duplication and fit within the sidebar * fix: ut

chore: implement new mockup to the new viz gallery

18f21f6

pull-request-size bot added the size/L label Jul 23, 2021

stephenLYZ changed the title ~~chore: implement new mockup to the new viz gallery~~ chore: implement new mockup to the new viz gallery (2nd iteration) Jul 23, 2021

stephenLYZ added 2 commits July 24, 2021 00:36

fix: update package-lock

86da059

fix: add license

0b0ec09

suddjian reviewed Jul 23, 2021

View reviewed changes

fix: reduce duplication and fit within the sidebar

d4b3f3c

stephenLYZ force-pushed the feature-gallery-2nd branch from 147bdd4 to d4b3f3c Compare July 24, 2021 12:51

fix: ut

6a49026

junlincc approved these changes Jul 26, 2021

View reviewed changes

suddjian approved these changes Jul 26, 2021

View reviewed changes

suddjian merged commit fb5dce0 into apache:master Jul 26, 2021

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 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: implement new mockup to the new viz gallery (2nd iteration) #15868

chore: implement new mockup to the new viz gallery (2nd iteration) #15868

stephenLYZ commented Jul 23, 2021 •

edited

Loading

suddjian Jul 23, 2021

stephenLYZ Jul 23, 2021

suddjian commented Jul 23, 2021 •

edited

Loading

suddjian commented Jul 23, 2021

github-actions bot commented Jul 23, 2021

stephenLYZ commented Jul 23, 2021

suddjian Jul 23, 2021

suddjian Jul 23, 2021

suddjian Jul 23, 2021

suddjian commented Jul 23, 2021

junlincc commented Jul 23, 2021

codecov bot commented Jul 24, 2021

github-actions bot commented Jul 25, 2021

suddjian commented Jul 26, 2021

github-actions bot commented Jul 26, 2021

kgabryje commented Jul 26, 2021

suddjian commented Jul 26, 2021

kgabryje commented Jul 26, 2021

junlincc commented Jul 26, 2021

junlincc left a comment

suddjian left a comment

github-actions bot commented Jul 26, 2021

chore: implement new mockup to the new viz gallery (2nd iteration) #15868

chore: implement new mockup to the new viz gallery (2nd iteration) #15868

Conversation

stephenLYZ commented Jul 23, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

suddjian Jul 23, 2021

Choose a reason for hiding this comment

stephenLYZ Jul 23, 2021

Choose a reason for hiding this comment

suddjian commented Jul 23, 2021 • edited Loading

suddjian commented Jul 23, 2021

github-actions bot commented Jul 23, 2021

stephenLYZ commented Jul 23, 2021

suddjian Jul 23, 2021

Choose a reason for hiding this comment

suddjian Jul 23, 2021

Choose a reason for hiding this comment

suddjian Jul 23, 2021

Choose a reason for hiding this comment

suddjian commented Jul 23, 2021

junlincc commented Jul 23, 2021

codecov bot commented Jul 24, 2021

Codecov Report

github-actions bot commented Jul 25, 2021

suddjian commented Jul 26, 2021

github-actions bot commented Jul 26, 2021

kgabryje commented Jul 26, 2021

suddjian commented Jul 26, 2021

kgabryje commented Jul 26, 2021

junlincc commented Jul 26, 2021

junlincc left a comment

Choose a reason for hiding this comment

suddjian left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 26, 2021

stephenLYZ commented Jul 23, 2021 •

edited

Loading

suddjian commented Jul 23, 2021 •

edited

Loading