tbonelee commented on PR #5074: URL: https://github.com/apache/zeppelin/pull/5074#issuecomment-3298618264
I missed that `componentFactoryResolver` could actually be passed into `JsonVisualization`. The reason I removed it was because no component in `src/app/visualizations` seemed to be using it, so I thought it was unused. For `JsonVisualization`, however, it seems better to restore it. That said, I was also considering whether we should remove the new Helium visualization interface that relies on Angular's dynamic rendering. My reasoning is twofold: 1. The Helium visualization packages currently published on npm are still based on the old UI (AngularJS) visualization class, so I don't think the new interface is practically in use. (`JsonVisualization` exists, but I see it more as an example than something used in practice.) 2. Even if we do provide a new Helium visualization interface, I think it would be better if it could be implemented without depending on Angular. That way, we could reduce framework-specific dependencies in the future. So my proposal is: - Add an adapter for the old Helium visualization packages (#5006). - Keep the new visualization logic only for the default visualizations for now (`src/app/visualizations`) - If a new Helium visualization interface is needed later, provide a framework-agnostic class instead of one tied to Angular. What do you think about this direction? -- 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]
