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/02/25 16:58:22 UTC
[GitHub] [superset] geido opened a new pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
geido opened a new pull request #18950:
URL: https://github.com/apache/superset/pull/18950
<!---
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 -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### 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 -->
- [ ] Has associated issue:
- [ ] 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 #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #18950:
URL: https://github.com/apache/superset/pull/18950#issuecomment-1056862371
Hello @zhaoyongjie this is the problem that I am mentioning in the description as well. This issue is affecting master too and it requires more investigation. @stephenLYZ is working on it in a separate PR. Thanks!
--
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] zhaoyongjie commented on pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #18950:
URL: https://github.com/apache/superset/pull/18950#issuecomment-1056897111
> Hello @zhaoyongjie this is the problem that I am mentioning in the description as well. This issue is affecting master too and it requires more investigation. @stephenLYZ is working on it in a separate PR. Thanks!
Thanks for the explanation. LGTM
--
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 #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #18950:
URL: https://github.com/apache/superset/pull/18950#issuecomment-1056878120
For reference, the problem mentioned above can also be reproduced from this PR https://github.com/apache/superset/pull/17726. The Sort descending will always be applied even when hidden
--
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 a change in pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on a change in pull request #18950:
URL: https://github.com/apache/superset/pull/18950#discussion_r817659926
##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
##########
@@ -508,6 +517,7 @@ const sharedControls = {
limit,
timeseries_limit_metric: enableExploreDnd ? dnd_sort_by : sort_by,
orderby: enableExploreDnd ? dnd_sort_by : sort_by,
+ order_desc,
Review comment:
Can we reuse all the chart's order desc with this? Thanks!
--
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 a change in pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18950:
URL: https://github.com/apache/superset/pull/18950#discussion_r822943570
##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
##########
@@ -508,6 +517,7 @@ const sharedControls = {
limit,
timeseries_limit_metric: enableExploreDnd ? dnd_sort_by : sort_by,
orderby: enableExploreDnd ? dnd_sort_by : sort_by,
+ order_desc,
Review comment:
As this discussed, this is a wider scope that it's best tackled in a separate PR.
--
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 a change in pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on a change in pull request #18950:
URL: https://github.com/apache/superset/pull/18950#discussion_r817659926
##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
##########
@@ -508,6 +517,7 @@ const sharedControls = {
limit,
timeseries_limit_metric: enableExploreDnd ? dnd_sort_by : sort_by,
orderby: enableExploreDnd ? dnd_sort_by : sort_by,
+ order_desc,
Review comment:
Can we replace all the chart's order desc with this? Thanks!
--
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 #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18950:
URL: https://github.com/apache/superset/pull/18950#issuecomment-1054488786
# [Codecov](https://codecov.io/gh/apache/superset/pull/18950?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 [#18950](https://codecov.io/gh/apache/superset/pull/18950?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e1ca34) into [master](https://codecov.io/gh/apache/superset/commit/2cb3635256ee8e91f0bac2f3091684673c04ff2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2cb3635) will **increase** coverage by `0.00%`.
> The diff coverage is `86.44%`.
> :exclamation: Current head 0e1ca34 differs from pull request most recent head 8b76318. Consider uploading reports for the commit 8b76318 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18950?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 #18950 +/- ##
=======================================
Coverage 66.38% 66.39%
=======================================
Files 1640 1641 +1
Lines 63514 63516 +2
Branches 6418 6423 +5
=======================================
+ Hits 42164 42171 +7
+ Misses 19690 19685 -5
Partials 1660 1660
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `51.00% <86.20%> (+<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/18950?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...perset-ui-chart-controls/src/utils/D3Formatting.ts](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3V0aWxzL0QzRm9ybWF0dGluZy50cw==) | `100.00% <ø> (ø)` | |
| [...reset-chart-deckgl/src/utilities/Shared\_DeckGL.jsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LWRlY2tnbC9zcmMvdXRpbGl0aWVzL1NoYXJlZF9EZWNrR0wuanN4) | `84.21% <ø> (ø)` | |
| [...acy-preset-chart-deckgl/src/utilities/controls.jsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LWRlY2tnbC9zcmMvdXRpbGl0aWVzL2NvbnRyb2xzLmpzeA==) | `11.11% <ø> (-8.89%)` | :arrow_down: |
| [...s/legacy-preset-chart-nvd3/src/Bar/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LW52ZDMvc3JjL0Jhci9jb250cm9sUGFuZWwudHM=) | `16.66% <0.00%> (-83.34%)` | :arrow_down: |
| [...gins/legacy-preset-chart-nvd3/src/NVD3Controls.tsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LW52ZDMvc3JjL05WRDNDb250cm9scy50c3g=) | `100.00% <ø> (+4.16%)` | :arrow_up: |
| [...c/SqlLab/components/RunQueryActionButton/index.tsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1J1blF1ZXJ5QWN0aW9uQnV0dG9uL2luZGV4LnRzeA==) | `50.00% <ø> (ø)` | |
| [...d/src/SqlLab/components/SaveDatasetModal/index.tsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NhdmVEYXRhc2V0TW9kYWwvaW5kZXgudHN4) | `71.42% <ø> (ø)` | |
| [...frontend/src/SqlLab/components/SaveQuery/index.tsx](https://codecov.io/gh/apache/superset/pull/18950/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=) | `57.89% <ø> (ø)` | |
| [...rc/SqlLab/components/ScheduleQueryButton/index.tsx](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NjaGVkdWxlUXVlcnlCdXR0b24vaW5kZXgudHN4) | `19.23% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/superset/pull/18950/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `33.15% <ø> (ø)` | |
| ... and [53 more](https://codecov.io/gh/apache/superset/pull/18950/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18950?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/18950?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 [2cb3635...8b76318](https://codecov.io/gh/apache/superset/pull/18950?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] geido merged pull request #18950: chore: Change Sort by and Sort descending controls for Time-series Bar Chart
Posted by GitBox <gi...@apache.org>.
geido merged pull request #18950:
URL: https://github.com/apache/superset/pull/18950
--
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