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