feat(explore): Move chart actions into dropdown #19446

kgabryje · 2022-03-30T19:20:41Z

SUMMARY

This PR is a stage 1 of chart header redesign.
It focuses on moving the action button into a dropdown menu to declutter the header.

List of changes:

move "download json/csv/image" buttons to "Download" submenu of actions dropdown
move "Set up a report" to dropdown if there's no report
move "Edit report", "Enable/disable report" and "Delete report" to "Manage the report" submenu of actions dropdown
move embed code button and share by email button to dropdown
move copy permalink button to the left side of the header, next to "favourite" button
refactor embed code popover to be a modal, change styling of modal's content

Final design available in discussion: #19099

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-03-30.at.21.13.39.mov

TESTING INSTRUCTIONS

Verify that all action in the dropdown menu work exactly the same as they did before moving them to dropdown

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
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

CC @kasiazjc

codecov · 2022-03-30T20:08:59Z

Codecov Report

Merging #19446 (28d0c0f) into master (6b136c2) will increase coverage by 0.00%.
The diff coverage is 60.26%.

@@           Coverage Diff           @@
##           master   #19446   +/-   ##
=======================================
  Coverage   66.57%   66.58%           
=======================================
  Files        1675     1676    +1     
  Lines       64092    64172   +80     
  Branches     6519     6525    +6     
=======================================
+ Hits        42672    42731   +59     
- Misses      19729    19742   +13     
- Partials     1691     1699    +8

Flag	Coverage Δ
javascript	`51.31% <60.26%> (+0.05%)`	⬆️

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

Impacted Files	Coverage Δ
...set-frontend/src/components/ModalTrigger/index.jsx	`100.00% <ø> (ø)`
...frontend/src/dashboard/components/Header/index.jsx	`60.92% <ø> (ø)`
...rc/explore/components/ExploreChartHeader/index.jsx	`50.00% <ø> (+1.31%)`	⬆️
.../components/ExploreAdditionalActionsMenu/index.jsx	`57.14% <55.00%> (-38.32%)`	⬇️
...nts/ExploreAdditionalActionsMenu/ExploreReport.tsx	`57.89% <57.89%> (ø)`
...ontend/src/explore/components/EmbedCodeContent.jsx	`76.66% <76.66%> (ø)`
...rset-frontend/src/components/ReportModal/index.tsx	`78.46% <100.00%> (-3.63%)`	⬇️
...set-frontend/src/components/ReportModal/styles.tsx	`87.50% <0.00%> (-12.50%)`	⬇️
...plugins/legacy-plugin-chart-heatmap/src/Heatmap.js	`0.00% <0.00%> (ø)`
... and 8 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 6b136c2...28d0c0f. Read the comment docs.

villebro · 2022-03-31T09:51:46Z

It feels strange that the "Share chart by email" and "Embed code" are in the dropdown, while the permalink button is in the header, as I assume people will consider them similar use cases. I can see myself clicking on the action button and wondering where the permalink item is. Would one of these make sense?

Making a "Share chart" submenu where we would have "Copy permalink", "Embed code" and "Share chart by email". We could potentially leave the permalink button where it is, but this way all the sharing/permalink-based functionality could at least be found in the same place
Add "Copy permalink" to the Action menu. This would cause some duplication, but then it would also be located close to the other similar functionality
Move the permalink button into the Action menu all together. This would have the benefit of decluttering the header even further (I'm not sure how common it is to copy permalinks)
Turn the permalink button a dropdown menu with the different sharing options (copy permalink, share via email and embed code)

kgabryje · 2022-03-31T09:54:43Z

It feels strange that the "Share chart by email" and "Embed code" are in the dropdown, while the permalink button is in the header, as I assume people will consider them similar use cases. I can see myself clicking on the action button and wondering where the permalink item is. Would one of these make sense?

Making a "Share chart" submenu where we would have "Copy permalink", "Embed code" and "Share chart by email". We could potentially leave the permalink button where it is, but this way all the sharing/permalink-based functionality could at least be found in the same place

Add "Copy permalink" to the Action menu. This would cause some duplication, but then it would also be located close to the other similar functionality

Move the permalink button into the Action menu all together. This would have the benefit of decluttering the header even further (I'm not sure how common it is to copy permalinks)

Turn the permalink button a dropdown menu with the different sharing options (copy permalink, share via email and embed code)

Thanks for comments. Personally I like the 1st option the most, but I guess @kasiazjc would probably want to chime in 🙂

kasiazjc · 2022-03-31T10:04:28Z

It feels strange that the "Share chart by email" and "Embed code" are in the dropdown, while the permalink button is in the header, as I assume people will consider them similar use cases. I can see myself clicking on the action button and wondering where the permalink item is. Would one of these make sense?

Making a "Share chart" submenu where we would have "Copy permalink", "Embed code" and "Share chart by email". We could potentially leave the permalink button where it is, but this way all the sharing/permalink-based functionality could at least be found in the same place

Add "Copy permalink" to the Action menu. This would cause some duplication, but then it would also be located close to the other similar functionality

Move the permalink button into the Action menu all together. This would have the benefit of decluttering the header even further (I'm not sure how common it is to copy permalinks)

Turn the permalink button a dropdown menu with the different sharing options (copy permalink, share via email and embed code)

Thanks for comments. Personally I like the 1st option the most, but I guess @kasiazjc would probably want to chime in 🙂

Good point @villebro! I agree, 1st option would work best. In that case we can remove the icon from the header So we don't duplicate the actions.

kgabryje · 2022-03-31T10:21:07Z

@kasiazjc @villebro Thanks for feedback, here's how it looks after moving copy permalink to menu 🙂

villebro

LGTM with a few minor comments

villebro · 2022-03-31T10:55:23Z

...nd/src/explore/components/ExploreAdditionalActionsMenu/ExploreAdditionalActionsMenu.test.tsx

 import * as chartAction from 'src/components/Chart/chartAction';
 import * as downloadAsImage from 'src/utils/downloadAsImage';
-import fetchMock from 'fetch-mock';
+import * as exploreUtils from 'src/explore/exploreUtils';


nit: importing * always makes me cringe, but in a test I guess it's acceptable 😛

I don't like that either, but I think that syntax is necessary to use sinon.spy

villebro · 2022-03-31T10:57:46Z

superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/index.jsx

+              <Menu.Item key={MENU_KEYS.EMBED_CODE}>
+                <ModalTrigger
+                  triggerNode={
+                    <span data-test="embed-code-button">{t('Embed code')}</span>


minor comment: I'd kinda prefer to see "Embed code" below "Share chart via email"

* feat(explore): Move chart actions to a dropdown menu * Fix tests and add some new ones * Add background color to embed code button * Fix cypress tests * Move copy permalink to actions menu * Remove unused function * Move share by email above embed code * Fix test * Fix test * Fix test * Fix test * Fix test

kgabryje requested review from villebro, geido and michael-s-molina March 30, 2022 19:20

pull-request-size bot added the size/XXL label Mar 30, 2022

kgabryje added 4 commits March 31, 2022 10:57

feat(explore): Move chart actions to a dropdown menu

c25ea11

Fix tests and add some new ones

ba4c40a

Add background color to embed code button

aac5a49

Fix cypress tests

0f10e44

kgabryje force-pushed the feat/explore-actions branch from ec1d468 to 0f10e44 Compare March 31, 2022 08:58

Move copy permalink to actions menu

740008d

Remove unused function

9636466

kgabryje mentioned this pull request Mar 31, 2022

feat(explore): Move timer, row counter and cached pills to chart container #19458

Merged

9 tasks

villebro approved these changes Mar 31, 2022

View reviewed changes

kgabryje added 6 commits March 31, 2022 13:50

Move share by email above embed code

ff8e3f1

Fix test

c2ab524

Fix test

80f0eb0

Fix test

52a3cba

Fix test

4cf45b1

Fix test

28d0c0f

kgabryje merged commit 1a1322d into apache:master Mar 31, 2022

villebro added lts-v1 and removed lts-v1 labels Apr 1, 2022

villebro mentioned this pull request Apr 6, 2022

feat: Export Chart to .xml format #14628

Closed

8 tasks

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

mistercrunch added the 🚢 2.0.0 label Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(explore): Move chart actions into dropdown #19446

feat(explore): Move chart actions into dropdown #19446

kgabryje commented Mar 30, 2022 •

edited

Loading

codecov bot commented Mar 30, 2022 •

edited

Loading

villebro commented Mar 31, 2022

kgabryje commented Mar 31, 2022

kasiazjc commented Mar 31, 2022

kgabryje commented Mar 31, 2022

villebro left a comment

villebro Mar 31, 2022

kgabryje Mar 31, 2022

villebro Mar 31, 2022

kgabryje Mar 31, 2022

feat(explore): Move chart actions into dropdown #19446

feat(explore): Move chart actions into dropdown #19446

Conversation

kgabryje commented Mar 30, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Mar 30, 2022 • edited Loading

Codecov Report

villebro commented Mar 31, 2022

kgabryje commented Mar 31, 2022

kasiazjc commented Mar 31, 2022

kgabryje commented Mar 31, 2022

villebro left a comment

Choose a reason for hiding this comment

villebro Mar 31, 2022

Choose a reason for hiding this comment

kgabryje Mar 31, 2022

Choose a reason for hiding this comment

villebro Mar 31, 2022

Choose a reason for hiding this comment

kgabryje Mar 31, 2022

Choose a reason for hiding this comment

kgabryje commented Mar 30, 2022 •

edited

Loading

codecov bot commented Mar 30, 2022 •

edited

Loading