feat: SIP-34 explore save modal

[mistercrunch]

Getting the explore save modal closer to SIP-34

RFC: SIP-34 doesn't really offer the "Create a new dashboard and add this chart into it" option and I wanted to preserve that. I did it with a checkbox + leaving the select empty, but unclear what the best way would be.

After (updated to latest commit)

Before

SIP-34

[Steejay]

@mistercrunch see north star design for add chart to new dashboard

For the After image: The text field under radio buttons would benefit by adding a title so that its clear what the field is for. See SIP-34 screenshot you posted. Also I think "Save Chart" is more direct than "Save a Chart". "Save a Chart" implies unnecessary ambiguity in the statement.

[mistercrunch]

@Steejay , I decided to use the "CreatableSelect" component feature from react-select to avoid having to build the select box.

I also improved the e2e cypress tests quite a bit, fixed and re-enabled tests that had been disabled.

[codecov-commenter]

Codecov Report

Merging #10355 into master will decrease coverage by 4.28%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master   #10355      +/-   ##
==========================================
- Coverage   69.70%   65.42%   -4.29%     
==========================================
  Files         196      603     +407     
  Lines       18950    32393   +13443     
  Branches        0     3287    +3287     
==========================================
+ Hits        13210    21194    +7984     
- Misses       5740    11015    +5275     
- Partials        0      184     +184     

Flag	Coverage Δ
#javascript	59.26% <95.00%> (?)
#python	69.75% <100.00%> (+0.04%)	⬆️

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

Impacted Files	Coverage Δ
...rset-frontend/src/explore/components/SaveModal.jsx	90.90% <95.00%> (ø)
superset/views/core.py	74.58% <100.00%> (+0.18%)	⬆️
superset/utils/pandas_postprocessing.py	76.96% <0.00%> (-12.98%)	⬇️
superset/errors.py	93.75% <0.00%> (-6.25%)	⬇️
superset/exceptions.py	95.12% <0.00%> (-4.88%)	⬇️
superset/views/sql_lab.py	58.62% <0.00%> (-3.96%)	⬇️
superset/models/sql_lab.py	91.12% <0.00%> (-1.62%)	⬇️
superset/views/base.py	72.50% <0.00%> (-0.92%)	⬇️
superset/viz.py	57.04% <0.00%> (-0.34%)	⬇️
superset/app.py	80.95% <0.00%> (-0.08%)	⬇️
... and 424 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 1a41ea4...32b0f52. Read the comment docs.

[rusackas]

@Steejay , I decided to use the "CreatableSelect" component feature from react-select to avoid having to build the select box.

I also improved the e2e cypress tests quite a bit, fixed and re-enabled tests that had been disabled.

The more I look at this the more I'm agonizing over the 🔘 Save (overwrite) 🔘 Save As choice.

Could it just be:

Save as:
[SomeChartame         ]
⚠️ Warning: This will overwrite the current *SomeChartame* chart

^ Just displaying an overwrite warning, rather than forcing a choice?

[ktmud]

Save as:
[SomeChartame         ]
⚠️ Warning: This will overwrite the current *SomeChartame* chart

^ Just displaying an overwrite warning, rather than forcing a choice?

I don't think this works as two charts can have the same title. Sometimes users just want to save a new copy of a chart and play with it.

I also feel CreatableSelect is not quite clear as the original radio buttons. Maybe keep it as is before we implement the SIP-34 design?

[mistercrunch]

@ktmud I don't have data to back this up, but the "save AND create a new dashboard" is not super common, it's still very much possible and discoverable IMHO.

I think the previous modal looks awful and is super confusing. I'd hate to shelf this because it's not 100% SIP-34. This PR also re-enables/improves many unit tests and removes a lot of unneeded complexity.

@ktmud if the concern is the "create dashboard" is not discoverable enough, I can think of ways to improve that, like bolding the words "Create" and "Select" in the placeholder of the dashboard dropdown

[mistercrunch]

Proposing two options:

• Bolded

• Capitalized and bolded

[Steejay]

I prefer option 1. All caps is a little too aggressive/dominating. Additionally, the header should read "Save Chart" not "Save A Chart". Can we add the title "Chart Name*" above the text field as well?

see comments in #10355 (comment)

[mistercrunch]

Latest:

[mistercrunch]

@ktmud I'd love your blessing on this, but I think this is solid mergeable progress and a good checkpoint in the right direction

[mistercrunch]

Latest, with the new control-label

[ktmud]

This design is definitely much cleaner than the existing layout. I'm OK with merging as is but would love to see the new iteration soon. A placeholder text still doesn't look as discoverable as the inline input in SIP-34 or the radio buttons in old design.

Another problem with current CreatableSelect is that it blocks users from creating a new dashboard with the same name as some existing dashboards.

[rusackas]

Impacts #8976