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/01/09 09:50:10 UTC
[GitHub] [superset] stephenLYZ opened a new pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
stephenLYZ opened a new pull request #17976:
URL: https://github.com/apache/superset/pull/17976
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Currently, the select component will place the selected option in front of the dropdown options by default, which is a convenient interaction. However, in some scenarios, the user may not want to do this( https://github.com/apache/superset/issues/15972), so we add an option to control this behavior.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### before
### after
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/issues/15972
- [ ] Required feature flags:
- [ ] 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] geido commented on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008899189
> @stephenLYZ @jinghua-qa One of the core requirements during the Select development was to standardize the behavior throughout the application. The previous component had so many configuration options that it seemed that we had a bunch of different selects throughout the application. Every time the user interacted with it the behavior was different. Having said that, I wonder what's @jess-dillard and @rusackas opinion on this particular requirement.
I agree with @michael-s-molina here. I believe we should not introduce this property as we end up creating inconsistency as it used to happen before the new Select component was introduced.
However, I think the problem here is that the list is not in alphabetical order independently of whether an option is selected.
--
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 #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008734689
@stephenLYZ @jinghua-qa One of the core requirements during the Select development was to standardize the behavior throughout the application. The previous component had so many configuration options that it seemed that we had a bunch of different selects throughout the application. Every time the user interacted with it the behavior was different. Having said that, I wonder what's @jess-dillard and @rusackas opinion on this particular requirement.
--
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 #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
jess-dillard commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1009241321
@stephenLYZ I agree with @michael-s-molina and @geido that we should keep the pattern consistent throughout the application. Michael's pagination use case makes sense as a reason to keep the current behavior (selected values appear first in the dropdown list). @stephenLYZ, I am curious what use cases you have in mind to support why this might not be the best behavior.
--
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 #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008274811
# [Codecov](https://codecov.io/gh/apache/superset/pull/17976?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 [#17976](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c0d193) into [master](https://codecov.io/gh/apache/superset/commit/cb97e37a92a3652d4a450678041acf4ae82e5566?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cb97e37) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17976 +/- ##
=======================================
Coverage 67.08% 67.08%
=======================================
Files 1611 1611
Lines 64919 64920 +1
Branches 6871 6872 +1
=======================================
+ Hits 43548 43549 +1
Misses 19504 19504
Partials 1867 1867
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.78% <100.00%> (+<0.01%)` | :arrow_up: |
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/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nts/controls/DateFilterControl/DateFilterLabel.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC9EYXRlRmlsdGVyTGFiZWwudHN4) | `57.65% <ø> (ø)` | |
| [superset-frontend/src/components/Select/Select.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1NlbGVjdC50c3g=) | `86.47% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb97e37...7c0d193](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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 edited a comment on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1009044225
> @michael-s-molina @geido Thanks for the explanations! I also agree keeping the behavior consistent. btw, I am curious why single-select component would keep selected option on top. It is a bit different from other site. Maybe scroll to selected option could be better? Since after each selection of value will lead to the reorder of the list, if the list is very large there maybe some performance issues.
We chose to keep the selected option on top to preserve the same behavior between the different types but also because the component is paginated, which means that all the results won't be available at a given point. Imagine a Select with the names of people and when you open it, only the names that start with A are available but the selected value is Sarah. In terms of performance, the virtualization and pagination help, we tested with more than 100k values and saw no problem.
--
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 edited a comment on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1009044225
> @michael-s-molina @geido Thanks for the explanations! I also agree keeping the behavior consistent. btw, I am curious why single-select component would keep selected option on top. It is a bit different from other site. Maybe scroll to selected option could be better? Since after each selection of value will lead to the reorder of the list, if the list is very large there maybe some performance issues.
We chose to keep the selected option on top to preserve the same behavior between the different types but also because the component is paginated, which means that all the results won't be available at a given point. Imagine a Select with the names of people and when you open it, only the names that start with A are available but the selected value is Sarah. In terms of performance, the virtualization and pagination helps, we tested with more than 100k values and saw no problem.
--
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 #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1017094607
Thanks for the PR @stephenLYZ. It does solve the issue reported. However, in the end, I agree with the general consensus of the thread: consistency is king. While the use case cited in the bug report does NOT require pagination, and I can see the reasoning for keeping the selected value in-place in that use case, I still tend to agree that we should not have different behaviors of Select components in the application. If we change our minds later, we can re-open this PR and rekindle this project (also adding it to Storybook if we do). In the meantime, I think we can close this. I think we can also close the original issue, but I'll take that up on that thread separately.
--
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] stephenLYZ commented on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1017098026
@rusackas Yeah. I think that makes sense. So let me close it.
--
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 #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1009044225
> @michael-s-molina @geido Thanks for the explanations! I also agree keeping the behavior consistent. btw, I am curious why single-select component would keep selected option on top. It is a bit different from other site. Maybe scroll to selected option could be better? Since after each selection of value will lead to the reorder of the list, if the list is very large there maybe some performance issues.
We chose to keep the select option on top to preserve the same behavior between the different types but also because the component is paginated, which means that all the results won't be available at a given point. Imagine a Select with the names of people and when you open it, only the names that start with A are available but the selected value is Sarah. In terms of performance, the virtualization and pagination helps, we tested with more than 100k values and saw no problem.
--
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] edited a comment on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008274811
# [Codecov](https://codecov.io/gh/apache/superset/pull/17976?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 [#17976](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c0d193) into [master](https://codecov.io/gh/apache/superset/commit/cb97e37a92a3652d4a450678041acf4ae82e5566?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cb97e37) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17976 +/- ##
=======================================
Coverage 67.08% 67.08%
=======================================
Files 1611 1611
Lines 64919 64920 +1
Branches 6871 6872 +1
=======================================
+ Hits 43548 43549 +1
Misses 19504 19504
Partials 1867 1867
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.78% <100.00%> (+<0.01%)` | :arrow_up: |
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/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nts/controls/DateFilterControl/DateFilterLabel.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC9EYXRlRmlsdGVyTGFiZWwudHN4) | `57.65% <ø> (ø)` | |
| [superset-frontend/src/components/Select/Select.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1NlbGVjdC50c3g=) | `86.47% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb97e37...7c0d193](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] stephenLYZ closed pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
stephenLYZ closed pull request #17976:
URL: https://github.com/apache/superset/pull/17976
--
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] edited a comment on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008274811
# [Codecov](https://codecov.io/gh/apache/superset/pull/17976?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 [#17976](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c0d193) into [master](https://codecov.io/gh/apache/superset/commit/cb97e37a92a3652d4a450678041acf4ae82e5566?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cb97e37) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17976 +/- ##
=======================================
Coverage 67.08% 67.08%
=======================================
Files 1611 1611
Lines 64919 64920 +1
Branches 6871 6872 +1
=======================================
+ Hits 43548 43549 +1
Misses 19504 19504
Partials 1867 1867
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.78% <100.00%> (+<0.01%)` | :arrow_up: |
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/17976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nts/controls/DateFilterControl/DateFilterLabel.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC9EYXRlRmlsdGVyTGFiZWwudHN4) | `57.65% <ø> (ø)` | |
| [superset-frontend/src/components/Select/Select.tsx](https://codecov.io/gh/apache/superset/pull/17976/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1NlbGVjdC50c3g=) | `86.47% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb97e37...7c0d193](https://codecov.io/gh/apache/superset/pull/17976?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] stephenLYZ closed pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
stephenLYZ closed pull request #17976:
URL: https://github.com/apache/superset/pull/17976
--
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] stephenLYZ commented on pull request #17976: feat(Select): [cosmetic] support allowTopOptions for select component
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on pull request #17976:
URL: https://github.com/apache/superset/pull/17976#issuecomment-1008951962
@michael-s-molina @geido Thanks for the explanations! I also
agree keeping the behavior consistent. btw, I am curious why single-select component would keep selected option on top. It is a bit different from other site. Maybe scroll to selected option could be better? Since after each selection of value will lead to the reorder of the list, if the list is very large there maybe some performance issues.
--
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