fix: Explore misleading save action

[michael-s-molina]

SUMMARY

When saving a chart in Explore, we have different states:

• If a user is overriding a chart or creating a new one
• If a user is selecting a dashboard or creating a new one
• If a user can override a chart or not

Currently, the Save modal buttons change to accommodate different combination of states:

• Save
• Save as new chart
• Save to new dashboard
• Save & go to dashboard
• Save & go to new dashboard

The problem is that are missing some combinations which leads to confusion when saving charts:

• Save as new chart to new dashboard
• Save as new chart & go to dashboard
• Save as new chart & go to new dashboard

One example of such confusion is when a user is not an owner of a chart but can save that chart as a new one. The button displays Save & go to dashboard which leads the user to think that a chart will be overridden in that dashboard but what actually happens is that a new chart is created and added to the dashboard. One can argue that the Save as radio selection would be sufficient to indicate the difference but that's not the case. Specially because we're changing the buttons text for some states and not for others.

To fix this, my first attempt was to make sure the button labels accounted for all possible states but that resulted in a poor UI as you can see bellow:

A better solution was to change the modal to preserve the button labels and add an information message when creating a new chart, a dashboard or both:

This message only appears when it's not clear that a new asset will be created. If a user is creating a chart for the first time and adding it to an existing dashboard, no message is displayed.

This PR also moves the alert message to the bottom, similar to the native filters modal, closer to the dashboard select and save button which are the sources for the alert.

TESTING INSTRUCTIONS

Make sure the correct information message is displayed or not depending on the chart state.

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

[justinpark]

These labels should be a good example of adding unit tests.

[michael-s-molina]

Yes but I'll focus on the PR changes. This part was only indented.

[kasiazjc]

I think this change makes the process more intuitive, so lgtm design wise. Thanks!

[codecov]

Codecov Report

Merging #24862 (7ac29d3) into master (77889b2) will increase coverage by 0.01%.
Report is 6 commits behind head on master.
The diff coverage is 91.15%.

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

@@            Coverage Diff             @@
##           master   #24862      +/-   ##
==========================================
+ Coverage   68.99%   69.01%   +0.01%     
==========================================
  Files        1903     1904       +1     
  Lines       74073    74111      +38     
  Branches     8193     8196       +3     
==========================================
+ Hits        51108    51145      +37     
- Misses      20844    20847       +3     
+ Partials     2121     2119       -2     

Flag	Coverage Δ
javascript	55.84% <77.41%> (+0.01%)	⬆️

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

Files Changed	Coverage Δ
...ins/legacy-preset-chart-nvd3/src/transformProps.js	7.14% <0.00%> (-0.27%)	⬇️
...t-frontend/src/explore/actions/saveModalActions.js	95.52% <ø> (-0.13%)	⬇️
...-frontend/src/explore/reducers/saveModalReducer.js	60.00% <ø> (+5.45%)	⬆️
superset/advanced_data_type/api.py	100.00% <ø> (ø)
superset/annotation_layers/annotations/api.py	89.23% <ø> (ø)
superset/annotation_layers/annotations/schemas.py	100.00% <ø> (ø)
superset/annotation_layers/api.py	86.20% <ø> (ø)
superset/annotation_layers/schemas.py	100.00% <ø> (ø)
superset/async_events/api.py	100.00% <ø> (ø)
superset/available_domains/api.py	100.00% <ø> (ø)
... and 58 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[justinpark]

LGTM