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

Redo ng2-konva in Angular 16 #54

Merged
merged 28 commits into from
Jul 27, 2023
Merged

Redo ng2-konva in Angular 16 #54

merged 28 commits into from
Jul 27, 2023

Conversation

haifishtime
Copy link
Contributor

Hey @rafaesc, @lavrton
as I already said I was working on migrating the project to Angular 16.

I kept the general logic of the original project the same but added types to pretty much all objects to better understand what's happening. For that reason I had to change some things to make it work again.

In the projects directory you can see two projects. ng2-konva is the actual lib and demo is a little angular application with all of the original demo snippets implemented.

What is still missing is the doc application. Maybe you can help on updating that one. I don't know if there is anything special about that site. If I see that correctly this is the site found here, correct?

Felix and others added 28 commits May 25, 2023 23:26
@haifishtime
Copy link
Contributor Author

haifishtime commented Jul 14, 2023

I created a local version of this package and refactored an application I'm working on to use this lib this week.
I am displaying an image, drawing a couple hundred rects onto it and listening to events to display a dynamic tooltip on hover.
I am happy to say I experienced no further issues related to this project.

@benjaminkech
Copy link

Thanks, @haifishtime! I refactored my Angular 16 app according to the newly introduced typings, making it compatible with Angular 16. This PR ensures that ng2-konva can continue to support Angular 16. @rafaesc and @lavrton, I kindly request your review and merge of this PR to maintain compatibility for other users. Thank you!

export class StageComponent
implements KonvaComponent, AfterContentInit, OnDestroy
{
private nodeContainer = inject(ElementRef).nativeElement;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When switching to the standard builder @angular-devkit/build-angular:browser, I encounter the following error: VM1600:1 ERROR Error: NG0203: inject() must be called from an injection context such as a constructor, a factory function, a field initializer, or a function used with runInInjectionContext. Find more at https://angular.io/errors/NG0203 at injectInjectorOnly (core.mjs:645:15) at ɵɵinject (core.mjs:656:60) at inject (core.mjs:740:12) at new StageComponent (stage.component.ts:31:33) at NodeInjectorFactory.StageComponent_Factory [as factory] (stage.component.ts:28:28)

I'm wondering if you are experiencing the same issue. @haifishtime

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just created a completely new app with angular 16 with ng new NAME, installed ng2-konva and konva, copied over one example and had no issues.
My new app is using @angular-devkit/build-angular:browser builder.
I'm pretty sure the code in the StageComponent is correct.
I can only imagine a misconfiguration or something on your site. If you can give me more info on how to replicate the issue I can try to help fix it.

@Jbz797
Copy link

Jbz797 commented Jul 25, 2023

Please merge

@lavrton lavrton merged commit a302df6 into konvajs:master Jul 27, 2023
@lavrton
Copy link
Member

lavrton commented Jul 28, 2023

I just released this version with next tag.

npm install ng2-konva@next

I need feedback and testing. Also, If possible, I am asking to create codesandbox examples. Something similar to examples in the repo: https://github.com/konvajs/ng2-konva/tree/master/projects/demo/src/app/examples
As minimal as possible, so we can drop working links in the README.
I am not familiar with Angular.

@haifishtime thanks a lot for your huge work.

@Jbz797
Copy link

Jbz797 commented Jul 31, 2023

So far it's working very well for me, thank you for your work.

@rmaleryk
Copy link

rmaleryk commented Aug 9, 2023

Was it really necessary to delete KonvaModule?

@haifishtime
Copy link
Contributor Author

Was it really necessary to delete KonvaModule?

To be honest it was probably not necessary to delete it but there is also no real point to keeping it.

It's just StageComponent and CoreShapeComponent which you need to import.

When I do another PR I will probably add it back as an option since there is no real disadvantage to having it.

@rmaleryk
Copy link

Was it really necessary to delete KonvaModule?

To be honest it was probably not necessary to delete it but there is also no real point to keeping it.

It's just StageComponent and CoreShapeComponent which you need to import.

When I do another PR I will probably add it back as an option since there is no real disadvantage to having it.

It is used to import these components into a custom module.
Without it, I need to import them into a component and make it standalone, which breaks the architecture of my application.
So I will be grateful if you return the module.

@haifishtime
Copy link
Contributor Author

There is no need for you to do anything standalone. You can just import the components into any module. Just like the KonvaModule. The only thing that has changed is that you need 2 imports instead of one.

import { NgModule } from '@angular/core';
import { CoreShapeComponent, StageComponent } from 'ng2-konva';

@NgModule({
  imports: [StageComponent, CoreShapeComponent]
})
export class AppModule {}

You might also need to export the two components depending on the usage of your module.

@rmaleryk
Copy link

@haifishtime I remember, I tried to import them to the module, but it didn't work. But now I tried one more time and seems like everything is ok.
Thanks!

haifishtime added a commit to haifishtime/ng2-konva that referenced this pull request Jan 20, 2024
Merge pull request konvajs#54 from haifishtime/master
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.

None yet

5 participants