feat: Allow mutli-select values to be edited

[reesercollins]

SUMMARY

When you mistype a filter value, there is no way to fix your mistake other than removing and re-adding it. This PR allows you to edit select component values by double-clicking on the filter tag.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

https://user-images.githubusercontent.com/10563996/183101208-8d4e29b3-e498-4b9b-bb56-ff8a1fde1f3d.mp4

TESTING INSTRUCTIONS

Add a filter value to a dashboard with the editable option enabled. Double click on the tag; an input should show up to allow you to modify the value. Hitting enter or clicking away from the input should save the value.

ADDITIONAL INFORMATION

• [ ] Has associated issue:
• [ ] Required feature flags:
• [x] 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

[codecov[bot]]

Codecov Report

Merging #20995 (0ac3a12) into master (0042ade) will decrease coverage by 0.03%. The diff coverage is 7.50%.

@@            Coverage Diff             @@
##           master   #20995      +/-   ##
==========================================
- Coverage   66.34%   66.30%   -0.04%     
==========================================
  Files        1767     1768       +1     
  Lines       67358    67398      +40     
  Branches     7147     7159      +12     
==========================================
+ Hits        44686    44689       +3     
- Misses      20844    20879      +35     
- Partials     1828     1830       +2     

Flag	Coverage Δ
javascript	52.06% <7.50%> (-0.06%)	:arrow_down:

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

Impacted Files	Coverage Δ
...c/filters/components/Select/SelectFilterPlugin.tsx	62.82% <ø> (ø)
...tend/src/filters/components/Select/controlPanel.ts	100.00% <ø> (ø)
...et-frontend/src/filters/components/Select/types.ts	100.00% <ø> (ø)
...set-frontend/src/components/Select/EditableTag.tsx	2.94% <2.94%> (ø)
...set-frontend/src/components/Select/AsyncSelect.tsx	82.62% <33.33%> (-0.64%)	:arrow_down:
superset-frontend/src/components/Select/Select.tsx	86.09% <33.33%> (-1.07%)	:arrow_down:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[michael-s-molina]

Hi @reesercollins. Thank you for your contribution.

I have some concerns with the current approach:

The most important one is that when editing a tag, you don't have access to the other options. If allowNewValue is false, you shouldn’t be able to change the value to something that does not exist. There's also the problem of typing a value that has not been loaded from the server-side when using an async select, which can result in a missing id for example.

Another problem is the input inside input pattern, which I don't think will be approved by the design team.

Personally, I don't think the delete/retype requires much effort. If that was the case, Ant Design would support editing a tag natively. I'm not sure if the added code complexity pays off in this case.

If you think this is really necessary we could start a discussion with the design team to see if there's an alternative way to edit the tags as long as we resolve the technical problems too.

Let me know your thoughts and thanks again for the PR. I really appreciate CCCS's contributions regarding this component.

[geido]

@reesercollins thanks for contribution! I agree with Michael about the potential issues of these changes, for both the technical implications and most importantly concerning the product. As much as I appreciate the effort, I believe we should reconsider the need for this enhancement.

[reesercollins]

Thank you for your feedback @michael-s-molina and @geido.

Our team does have a common use case where deleting retyping filter values can be quite tedious (i.e. long domain names) where you may have just made a single typo.

Regarding preventing new values from being entered when they shouldn't be, I have made editing only functional when allowNewValue is true.

We are open to revamping the design. This was just my first attempt at a working version.

[michael-s-molina]

Our team does have a common use case where deleting retyping filter values can be quite tedious (i.e. long domain names) where you may have just made a single typo.

That's a good example to help us better understand your use case. I was thinking about the problem, and a possible idea would be to double-click on the tag and turn it back to the input with the value filled in. That way it would be the same behavior as if you just typed, with search and loading capabilities.

Pinging @jess-dillard and @kasiazjc to help us here.

[rusackas]

Pinged @kasiazjc for some added design input on this.

[ghost]

a possible idea would be to double-click on the tag and turn it back to the input with the value filled in. That way it would be the same behavior as if you just typed, with search and loading capabilities.

This was my thought for a more elegant solution as well. It's important this functionality is only accessible when allowNewValue is true.

[reesercollins]

After attempted the proposed solution, I ran into issues with the ant design select component. Trying to edit the value of the input using the searchValue prop causes the component to no longer fine the onSearch callback for some reason. I can’t figure out exactly why, but this seems to be an ant design problem, not superset.

[rusackas]

Has anyone assessed if there's a newer version of AntD that might resolve this issue? We're looking at upgrading it (just a minor bump) but it reminded me of this lingering issue.