You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Vitor-Avila (via GitHub)" <gi...@apache.org> on 2024/01/21 00:23:39 UTC
[PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Vitor-Avila opened a new pull request, #26705:
URL: https://github.com/apache/superset/pull/26705
### SUMMARY
The Time Grain control is no longer visible for legacy charts in case `GENERIC_CHART_AXES` is enabled.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before**
![image](https://github.com/apache/superset/assets/96086495/2c3ec066-9ce1-44af-a7d8-0a6035813d4b)
**After**
![image](https://github.com/apache/superset/assets/96086495/573d1264-e45d-4304-b4aa-826195a90d6a)
### TESTING INSTRUCTIONS
1. Make sure that `GENERIC_CHART_AXES` is disabled.
2. Create a legacy line chart with a temporal column and a time grain applied.
3. Enable `GENERIC_CHART_AXES` and access the chart.
4. Make sure that the Time Grain field is still visible.
I also created charts using all nvd3 charts + all viz types using `legacyTimeseriesTime` and confirmed that changing the FF value did not introduced any issues, and the time grain field was visible and working.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `GENERIC_CHART_AXES`
- [ ] 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
Re: [PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #26705:
URL: https://github.com/apache/superset/pull/26705#issuecomment-1902464673
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `1 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`e86d4d3`)](https://app.codecov.io/gh/apache/superset/commit/e86d4d3c92b91da0aa09bc215908c23f261b4f9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 69.48% compared to head [(`db3f6fb`)](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 69.48%.
| [Files](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rt-controls/src/shared-controls/sharedControls.tsx](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9zaGFyZWRDb250cm9scy50c3g=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #26705 +/- ##
=======================================
Coverage 69.48% 69.48%
=======================================
Files 1894 1894
Lines 74138 74138
Branches 8243 8243
=======================================
Hits 51515 51515
Misses 20554 20554
Partials 2069 2069
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/26705/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/26705/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.86% <0.00%> (ΓΈ)` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/26705?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #26705:
URL: https://github.com/apache/superset/pull/26705#issuecomment-1908583930
@Vitor-Avila this looks like a really clever solution, kudos for the ingenuity! π
--
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
Re: [PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila commented on PR #26705:
URL: https://github.com/apache/superset/pull/26705#issuecomment-1908613150
> @Vitor-Avila this looks like a really clever solution, kudos for the ingenuity! π
Thanks @villebro! Hopefully this helps getting more Orgs unblocked to enabling `GENERIC_CHART_AXES` without having to dedicate a lot of effort in updating legacy charts, so that instead we can focus on migrating them to Echarts π
--
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
Re: [PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #26705:
URL: https://github.com/apache/superset/pull/26705
--
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
Re: [PR] fix(legacy-charts): Show Time Grain control for legacy charts [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #26705:
URL: https://github.com/apache/superset/pull/26705#issuecomment-1910303656
Thank you @Vitor-Avila!
--
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