Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat) propose new release v5.0.0 #392

Open
wants to merge 42 commits into
base: master
Choose a base branch
from

Conversation

medbenmakhlouf
Copy link

I had the motivation to update the package "highcharts-angular", including the demo app, to the latest version of Angular to take advantage of new reactivity features, now stable as of version 19.

This update will include:

  • Replacing lifecycle hooks with Signals for better state management.

  • Introducing Input and Output Signals for component communication.

  • Utilizing computed and LinkedSignal for derived state.

  • Applying effect to handle side effects efficiently.

By adopting these changes, we can ensure our components are more reactive, performant, and easier to maintain in the long term, with improved test coverage to support the updates.

@PowerKiKi
Copy link

That looks like quite an improvement. What aboud getting rid of the module once and for all, since we are breaking the lib anyway ?

Also, since we are doing a major version, I would strongly suggest to incorporate those suggestions in one way or another. The purpose of that is to make this lib effortlessly lazy load Highchart by default, and to be able to load extra optional features easily in a typed manner. All of this becomes natural if we replace the input for Highcharts by a provideHighcharts() function. What do you think ?

@medbenmakhlouf
Copy link
Author

@PowerKiKi, with my latest commits, I was able to move the code from the Highcharts Component to a directive and use that directive as host directives. Now developers can have two different ways to develop their charts.

On the other hand, I saw your comment about lazy-loading the library, and here I tried to come up with an approach using the preferred syntax we see these days in the Angular ecosystem.

I have developed the provideHighChartsModuleFactory() to help lazy-load Highcharts with modules like Gantt, Stock, Map, etc., on demand.

take a look at the demo example and let me know ;)

@PowerKiKi
Copy link

have two different ways to develop their charts

Can you elaborate on what is the advantage of having a directive ? It seems to me that a Highcharts charts fits especially well the concept of component, and it feels rather awkward as a directive because it replace the entire content of the element where the directive is applied. What real world use-case could be solved with a directive that cannot be solved with the existing component ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants