amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096214540
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
Thanks for the questions :
1. We do need seperate visualisations since the original table has alot of
time comparison features & already so much legacy code in place that rewriting
the entire legacy table with ag grid can break the backward compatibility for
existing clients , it might be possible in future to completely migrate to ag
grid & discard the old table but as of now , i think this carries risk seeing
the complexity of existing table chart , given that table is one of the most
widely used charts
2. Good point - even i think table_v2 would be better name , just needed a
consensus from the pmcs
3.I thought the same thing in the first place , but the thing is, it will
break the principle of loose coupling , sqllab & viz plugins have least common
functionalities & to implement any functionality in viz plugin table v2 might
cause implication in sqllab component where i have to keep on passing flags to
check where isSqllab or isVizPlugin , so i implemented it from scratch so that
its losely coupled & any changes in this component doesnt break in sqllab or
sqlllab changes doesnt break viz plugin
@michael-s-molina @justinpark @sadpandajoe
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
