You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/08/05 14:46:12 UTC
[GitHub] [superset] reesercollins opened a new pull request, #20995: feat: Allow mutli-select values to be edited
reesercollins opened a new pull request, #20995:
URL: https://github.com/apache/superset/pull/20995
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### 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. 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
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] reesercollins commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
reesercollins commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1270366551
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina closed pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
michael-s-molina closed pull request #20995: feat: Allow mutli-select values to be edited
URL: https://github.com/apache/superset/pull/20995
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] reesercollins commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
reesercollins commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1209528661
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] jess-dillard commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
jess-dillard commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1230539078
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1291238123
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1209378883
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1229001654
Pinged @kasiazjc for some added design input on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
geido commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1209483372
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1206861186
# [Codecov](https://codecov.io/gh/apache/superset/pull/20995?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#20995](https://codecov.io/gh/apache/superset/pull/20995?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45d3ed3) into [master](https://codecov.io/gh/apache/superset/commit/eb5369f2a6f2dc238838119eb70194bf2b42b085?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eb5369f) will **decrease** coverage by `0.06%`.
> The diff coverage is `23.68%`.
> :exclamation: Current head 45d3ed3 differs from pull request most recent head b94c8f0. Consider uploading reports for the commit b94c8f0 to get more accurate results
```diff
@@ Coverage Diff @@
## master #20995 +/- ##
==========================================
- Coverage 66.38% 66.31% -0.07%
==========================================
Files 1767 1768 +1
Lines 67232 67331 +99
Branches 7138 7155 +17
==========================================
+ Hits 44633 44652 +19
- Misses 20773 20848 +75
- Partials 1826 1831 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `52.07% <30.18%> (-0.04%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/20995?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...set-ui-core/src/ui-overrides/ExtensionsRegistry.ts](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdWktb3ZlcnJpZGVzL0V4dGVuc2lvbnNSZWdpc3RyeS50cw==) | `100.00% <ø> (ø)` | |
| [...frontend/src/SqlLab/components/SaveQuery/index.tsx](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NhdmVRdWVyeS9pbmRleC50c3g=) | `74.35% <ø> (ø)` | |
| [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | `67.60% <0.00%> (-1.41%)` | :arrow_down: |
| [...ols/MetricControl/AdhocMetricEditPopover/index.jsx](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIvaW5kZXguanN4) | `75.53% <ø> (ø)` | |
| [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `62.82% <ø> (ø)` | |
| [...tend/src/filters/components/Select/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvY29udHJvbFBhbmVsLnRz) | `100.00% <ø> (ø)` | |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `100.00% <ø> (ø)` | |
| [superset-frontend/src/utils/datasourceUtils.js](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2RhdGFzb3VyY2VVdGlscy5qcw==) | `100.00% <ø> (ø)` | |
| [.../src/views/CRUD/data/dataset/DatasetPage/index.tsx](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0RhdGFzZXRQYWdlL2luZGV4LnRzeA==) | `0.00% <0.00%> (ø)` | |
| [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `39.40% <6.66%> (-1.38%)` | :arrow_down: |
| ... and [9 more](https://codecov.io/gh/apache/superset/pull/20995/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #20995: feat: Allow mutli-select values to be edited
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20995:
URL: https://github.com/apache/superset/pull/20995#issuecomment-1209753238
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org