feat(explore): make dnd controls clickable

[kgabryje]

SUMMARY

Changes:

• Clickable ghost button for metrics and filters controls. Clicking a ghost button opens a popover without any fields pre-filled, which means user opts out of UX improvements introduced with drag and drop. Dnd works exactly as before.
• A new popover for columns control, similar to a popover for metrics and filters. Contains 2 tabs - "Saved" with a select button for choosing calculated columns and "Simple" with a button for selecting normal columns.
• Clickable ghost button and clickable labels in columns control. Clicking a ghost button opens an empty popover in which user can select a column. Clicking a label opens a popover prefilled with that label. Dnd works as before - column is added without using popover. Dnd is the preferred way of adding columns as it requires much less user input - 1 drag versus 4 clicks (open popover, click select button, select column, click save).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-08-06.at.17.49.02.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

CC @junlincc @mistercrunch

[codecov]

Codecov Report

Merging #16119 (36a6be8) into master (9876c36) will decrease coverage by 0.21%.
The diff coverage is 78.94%.

@@            Coverage Diff             @@
##           master   #16119      +/-   ##
==========================================
- Coverage   76.71%   76.49%   -0.22%     
==========================================
  Files         997      997              
  Lines       53248    53248              
  Branches     6779     6779              
==========================================
- Hits        40849    40733     -116     
- Misses      12169    12285     +116     
  Partials      230      230              

Flag	Coverage Δ
hive	?
mysql	81.56% <ø> (-0.05%)	⬇️
postgres	81.62% <ø> (ø)
presto	?
python	81.71% <ø> (-0.43%)	⬇️
sqlite	81.23% <ø> (-0.05%)	⬇️

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

Impacted Files	Coverage Δ
superset/config.py	91.30% <ø> (ø)
...ontrols/DndColumnSelectControl/DndMetricSelect.tsx	41.21% <57.14%> (ø)
...ontrols/DndColumnSelectControl/DndFilterSelect.tsx	45.07% <66.66%> (ø)
...ontrols/DndColumnSelectControl/DndColumnSelect.tsx	46.42% <100.00%> (ø)
...controls/DndColumnSelectControl/DndSelectLabel.tsx	77.27% <100.00%> (ø)
superset/db_engines/hive.py	0.00% <0.00%> (-82.15%)	⬇️
superset/db_engine_specs/hive.py	69.80% <0.00%> (-16.87%)	⬇️
superset/db_engine_specs/presto.py	83.47% <0.00%> (-6.49%)	⬇️
superset/views/database/mixins.py	81.03% <0.00%> (-1.73%)	⬇️
superset/connectors/sqla/models.py	88.04% <0.00%> (-1.66%)	⬇️
... and 7 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 9876c36...36a6be8. Read the comment docs.

[villebro]

Minor first pass comments, will look more closely after I have the chance to test properly

[junlincc]

❤️ it as it's solid good work! as an OSS project, inclusiveness, flexibility and extensibility are always more important than chasing after perfect UX for a certain group of user regardless its size. :) I will help testing

[villebro]

Code LGTM, tested to work properly and I really like the functionality vs how it was previously. Looking forward to getting this merged!

[kgabryje]

CC @michellethomas

[kgabryje]

/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true

[github-actions]

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

[michellethomas]

Seems like a good improvement to me 👍 I tested this a bit and didn't see any issues but I only tested with a couple of datasources so I can look around a bit more later. cc: @ktmud you also looked around at drag and drop, not sure if you have any thoughts about this change.

[ktmud]

Didn't have time to look at the code but this change is a big + from me!

We might want to change the copy on the ghost button as well. E.g. "Click or drag a metric/column here to add" like used in SIP-62.

[kgabryje]

/testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_DND_WITH_CLICK_UX=true

[github-actions]

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

[kgabryje]

I've hidden the new features behind a new feature flag ENABLE_DND_WITH_CLICK_UX.

@ktmud We recently added a suffix "here" to the ghost buttons

However, if there are more votes for a longer text "Click or drag a metric/column here to add" I can change that.

@junlincc WDYT?

[villebro]

I would prefer "Click or drag a metric/column here" because:

• "to add" feels obvious in this context (I'm not sure what else the user could expect to happen by clicking or dragging a column/metric into the control)
• The string is already long, and in some other language it is more likely to overflow if we add more words.

[junlincc]

It depends on which direction we want to guide user to be on.
Here's my concern. With the + sign visual indication and the "Click or drop a metric/column here" placeholder, I am almost 100% sure first time users will click, instead of visit the left data panel first as their first interaction with this popover.
Once user start on one path, and find it sufficient enough, it's very likely they stick with the more familiar path and reluctant to try the other, which may slowdown the adoption of dnd.
The fact that DND offers a much fast time to chart, compared to click-to-select, I do want to intentionally guide user to start with this path, and promote this path.

I prefer to keep it simple"Drop a metric/column here" as the "+" is a loud enough call to action for clicking.
If we have to explicitly indicate again, "Click or drop a metric/column here" makes more sense to me, definitely not to use "drag" in the wording.

Also, recently we do identify some sever regressions in the control panel due to increasing complexity of the product by adding too many FF and having multiple environments. I can't emphasize enough that maintaining both and making sure both work well together and individually can be costly and messy.
I would push much harder keeping DND only and try to resolve all the existing concerns, bottlenecks by improving this one solution if I am working on a close source/design heavy product. However, Superset has its unique OSS DNA which should be preserved and respected.

To whomever approve and support this hybrid solution, please do assess the code, and test this PR throughout, and consider the risk, the complexity for long term. @ktmud @villebro

[ktmud]

Maybe promote DnD before click: "Drop a column/metric here or click"?

[junlincc]

Product sign-off. will do throughout testing next week behind FF

[rusackas]

Is this styling something we'd anticipate reusing anywhere? If so, maybe it should be styled via a prop on the Select component itself.

[michael-s-molina]

@kgabryje @rusackas This code is not using the new Select component. It's using the Ant Design directly. We shouldn't use the Ant Design component directly anymore because the behavior of the select is different, like sorting the selected options, pagination, and lazy loading. To customize the options of the new Select you can just pass the element in the label prop of the values.

These PRs contain an example of customized options:
#16200
#16252

I don't think this style should be a prop of the Select component because it's something specific to the customized label. We have many different customized labels throughout the application as you can see in the PRs above.

[ktmud]

SIMPLE makes sense in the context of metrics because it's a "simple" way of configuring adhoc metrics. But for columns, there is nothing to configure so the "simplicity" of selecting a calculated column and an original column is the same.

I wonder whether we can just display a popover without tabs but render "Columns" and "Calculated columns" as options groups. Then add the two tabs "SIMPLE" and "Custom SQL" when we add support for adhoc calculated columns.

[villebro]

Ping @michael-s-molina about proposal to add support for Option Groups to the Select component.

[michael-s-molina]

Code LGTM. I added the ColumnSelectPopover to our list of components that need to be migrated to the new Select.

[kgabryje]

@ktmud @junlincc @villebro I changed the ghost button texts to "Drop columns/metrics here or click" (and similar for singular version and columns only version)

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.

[henryyeh]

🏷️ 2021.31

[mdeshmu]

💯 I really appreciate that this is being added.