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

TypeScript and Angular 5 support #82

Closed
kartikbb opened this issue Apr 27, 2018 · 64 comments
Closed

TypeScript and Angular 5 support #82

kartikbb opened this issue Apr 27, 2018 · 64 comments
Labels
Projects

Comments

@kartikbb
Copy link

Do you plan to introduce TypeScript and Angular 5 support anytime in near future?

@dongsik-yoo dongsik-yoo added the Enhancement Enhance performance or improve usability of original features. label Apr 28, 2018
@dongsik-yoo
Copy link
Contributor

We'll discuss about your question.

@dongsik-yoo dongsik-yoo added Question and removed Enhancement Enhance performance or improve usability of original features. labels Apr 29, 2018
@MRisto-zz
Copy link

That would be great. This calendar rocks.

@dongsik-yoo
Copy link
Contributor

Thanks @mristo for your attention.
There are several request for supporting frameworks like vue, typescript.
I agree with you.
We don't have any plan, but we want it also.
Some typescript expert's help would be great for this calendar.

@amanvishnani
Copy link

Hi, I have started working on this issue as a hobby project. But ran into some CSS issues. I have created a trim down version to demonstrate it in a Typescript only project (not angular).
Request you to have a look at it.

@dongsik-yoo
Copy link
Contributor

I'm on it.

@dongsik-yoo
Copy link
Contributor

Oh, I found something wrong in the minified tui-calendar.min.css file.
Please use not minified css file tui-calendar.css instead until i fix it.
The next version might be released before 17th May.

Thank you!

@amanvishnani
Copy link

@dongsik-yoo Cool, I have updated it as directed. Now I don't see day numbers. The same link please check.

@dongsik-yoo
Copy link
Contributor

@amanvishnani Good, the only what you have to do is set height of container.
And in that link, there is no real #cal element before new Calendar().

@amanvishnani
Copy link

@dongsik-yoo Thanks for the help.

@dongsik-yoo
Copy link
Contributor

@amanvishnani You're welcome. Are you working on making typescript wrapper?

@amanvishnani
Copy link

@dongsik-yoo My primary goal is to make it work on Angular with Typescript by creating a component wrapper around it, which could be reused by installing as npm dependency and, importing module and components from it.

@dongsik-yoo
Copy link
Contributor

@amanvishnani Great! It will be awesome! I'm look forward to your good package.

@ghost
Copy link

ghost commented May 9, 2018

hello all, working on something similar, available at https://github.com/brnrds/ngx-tui-dev/tree/master/projects/ngx-tui-calendar/src/lib or via $ npm install ngx-tui-calendar

currently broken, getting

ERROR Error: StaticInjectorError(AppModule)[NgxTuiCalendarComponent -> ElementRef]:
StaticInjectorError(Platform: core)[NgxTuiCalendarComponent -> ElementRef]:
NullInjectorError: No provider for ElementRef!

in Angular 5/6 @amanvishnani maybe we can merge these efforts?

@amanvishnani
Copy link

@brnrds Sure we can work collaboratively on this project, I was hoping if we could host repository somewhere on this organization itself. This will keep all related repositories at one place.

@dongsik-yoo What do you think?

@dongsik-yoo
Copy link
Contributor

@brnrds @amanvishnani Amazing.
We have an internal process of managing github repository. I hope we offer a proper repository, but I can't promise you. We'll discuss this tomorrow. I'll let you know the result or maybe guide.

@ghost
Copy link

ghost commented May 9, 2018

@amanvishnani @dongsik-yoo so far my progress has been to emulate the approach in https://github.com/mattlewis92/angular-gauge/blob/master/src/gauge.component.ts to make a wrapper for tui.calendar.

@amanvishnani feel free to submit pull requests, or move the code in the repo above to a new one.

@dongsik-yoo
Copy link
Contributor

@brnrds @amanvishnani Good news!
We can offer a new repository for your working!
So what do you want the repository name to be?

@icepeng
Copy link

icepeng commented May 10, 2018

https://github.com/xieziyu/ngx-echarts/tree/master/projects/ngx-echarts/src/lib

Code inside this lib will help you a lot. It is Angular wrapper of Echarts, which provides directive and service to use echart. It shows how to handle window resize, option changes, and other events.

It would be also useful to wrap tui.chart with this approach.

@icepeng
Copy link

icepeng commented May 10, 2018

And using directive would be better than wrapper component, because getting viewport from DOM does not work well inside component.

https://github.com/nhnent/tui.calendar/blob/b908747419cb01caf23b5b8e838e00284650da2a/src/js/view/view.js#L182-L193

So these lines should not work as expected... you might have to manually pass height and width. Or you need a bunch of glue code for setting component's host style.

@dongsik-yoo ngx-<lib name> is generally used after Angular 4.x, so ngx-tui.calendar would be ok. But I prefer tui.calendar.ng or tui.calendar.ngx.

@amanvishnani
Copy link

I am in favor of either one of tui.calendar.ngx or ngx-tui-calendar as a repository name and @nhnet/<final-repo-name> as the final package name.
@dongsik-yoo @icepeng @brnrds what do you guys think?

@icepeng
Copy link

icepeng commented May 10, 2018

It might be @tui/calendar with scoped package. NHN Entertainment has other projects like Haste Framework, and tui means TOAST UI.

And we can create @tui/angular then implement tui components as much as possible. I think we don't need to create each repo like ngx-tui-calendar, ngx-tui-chart, ngx-tui-editor, etc... Just include Angular parts together in one module, and install scripts separately.

If you write angular project with tui.calendar and tui.chart:

package.json

"dependencies": {
  ~~~
  "@tui/angular": "1.0.0",
  "@tui/calendar": "1.0.0",
  "@tui/chart": "1.0.0"
}

angular.json

~~~
"styles": [
  "../node_modules/@tui/calendar/tui-calendar.min.css",
  "../node_modules/@tui/chart/tui-chart.min.css"
],
"scripts": [
  "../node_modules/@tui/calendar/tui-calendar.min.js",
  "../node_modules/@tui/chart/tui-chart.min.js"
]

app.module.ts

  imports: [
    ~~~
    TuiModule.forRoot()
  ]

app.component.html

<tui-calendar [options]="calendarOptions"></tui-calendar>
<tui-chart [options]="chartOptions"></tui-chart>
<!-- <tui-editor></tui-editor> (Error!) -->

@ghost
Copy link

ghost commented May 10, 2018

Thanks for the pull request, @amanvishnani ! Also for the reference @icepeng :-) Busy with a few tasks but I'll push an update as soon as I figure out how to actually show the calendar.

As for the repository name, while all suggestions seem viable, I particularly like '@tui/...' because it would make things organized as more Toast UI projects have their Angular wrappers. My suggestion is to keep working for now on the repository @amanvishnani is already contributing to, and once it's actually working create to "official" repository under your organization, and I'll delete this temporary one.

Also, input on best practices is most welcome, since I'm a relative newcomer to Angular (and client-side in general) and would like to contribute to a good implementation that can be used as reference for other TUI Angular wrappers.

@amanvishnani
Copy link

Completely agree with @icepeng suggestions.
@brnrds Sure let's work on your repo and once we a complete working prototype lets move it to the repo in this Org.

@dongsik-yoo
Copy link
Contributor

@icepeng As you point, tui-calendar's container must have height.

@amanvishnani @brnrds We have a discussion to determine the repository name and package name convention. I'll let you know when I have a decision.

@ghost
Copy link

ghost commented May 11, 2018

Hello all, things just got rather busy around here and I'll only have time/availability to work on this from the 2018/05/21 onward. If @amanvishnani or other contributors send pull requests I'll gladly merge them, or I can transfer ownership, whichever there is consensus about.

@ghost
Copy link

ghost commented May 18, 2018

Good news, had a chance to pick this up yesterday, and António, a colleague, also helped out. This is now working: https://github.com/brnrds/ngx-tui-dev , pending adding type/interface definitions for the library's classes and installation instructions.

@worthy7
Copy link

worthy7 commented Jun 22, 2018

@dongsik-yoo Thanks for the reply.

  1. I suppose the repository doesn't really matter. The main thing is just the npm package name - which is good at the moment.

  2. I think that primarily, just work with demand. At the moment this package is getting popular, so starting here with Typescript interfaces would be great. Of course if the whole thing was in Typescript, that would be better but it is too much to ask.

Thanks!

@youjung-hong youjung-hong added this to To do in Features via automation Jun 29, 2018
@dongsik-yoo
Copy link
Contributor

NHN Entertainment offers a new repository for angular wrapper. https://github.com/nhnent/tui.ngx-calendar. Thank you @kartikbb @mristo @amanvishnani @brnrds @worthy7 @icepeng @srinek .

And another TOAST UI Family, Grid v3.0 has been released. New awesome theme and Tree+Grid feature. Please take a look. This grid has beed adopted by many company at their back office.

tui.grid

@icepeng
Copy link

icepeng commented Jul 4, 2018

Good news.

If we create other Angular wrapper for TOAST UI, then we have to create each repositories like tui.ngx-grid, tui.ngx-chart... right?

I prefer to have only one package tui.angular and import module, like Angular Material does.

@worthy7
Copy link

worthy7 commented Jul 4, 2018

Hmm nah. Angular Material is going to rely on a lot of common base components, TUI doesn't.
We can change things in the future but I think we should at least try to continue like this for now until something difficult comes along.
For example:
https://github.com/angular/material2/blob/master/src/lib/tabs/tabs-module.ts

If one uses Angular Material then it's expected that you would use most of the modules. It's not like that with TUI.

@icepeng
Copy link

icepeng commented Jul 4, 2018

I thought too many repos would be created in nhnent organization in future, but it might be ok for now.

@worthy7
Copy link

worthy7 commented Jul 9, 2018

@brnrds Do you have write access to the new repo? Can you deal with my PR's please?

@dongsik-yoo
Copy link
Contributor

@worthy7 I merged your PR.

@worthy7
Copy link

worthy7 commented Jul 9, 2018 via email

@dongsik-yoo
Copy link
Contributor

@worthy7 Did you get the npm access permission from @brnrds ?
There is possibility that I make a mistake because I don't know about this wrapper.
I think it's better that you and @brnrds release the npm package.
What do you think?

@worthy7
Copy link

worthy7 commented Jul 9, 2018 via email

@vindiezel23
Copy link

ngx-tui-calendar-error

@vindiezel23
Copy link

vindiezel23 commented Jul 20, 2018

Anyone knows why I am getting the errors above?

@worthy7 @dongsik-yoo

@worthy7
Copy link

worthy7 commented Jul 20, 2018

@vindiezel23

  1. Please open these issues in the ngx repo
  2. The latest npm is broken @dongsik-yoo

@dongsik-yoo
Copy link
Contributor

@vindiezel23 The wrappers of tui-calendar are maintained by community and users. So ngx-tui-calendar's users will be grateful for you if you send us PR.
@worthy7 I'm sorry but I can't maintain the ngx repo because NHNEnt offers the ngx repo by request of users.

@worthy7
Copy link

worthy7 commented Jul 20, 2018

@dongsik-yoo Well, someone needs to be in charge of pushing and fixing NPM packages.

@dongsik-yoo
Copy link
Contributor

@brnrds Could please write about release process of npm package?
@worthy7 I agree. I ask @brnrds to provide the release process.

@worthy7
Copy link

worthy7 commented Aug 1, 2018

@brnrds poke

@worthy7
Copy link

worthy7 commented Aug 1, 2018

@dongsik-yoo Can you please give me NPM permission for now and I'll see what I can do

@worthy7
Copy link

worthy7 commented Aug 1, 2018

@dongsik-yoo
Copy link
Contributor

Done. @worthy7 has been added as a maintainer.

@worthy7
Copy link

worthy7 commented Aug 8, 2018

@dongsik-yoo How do you want this to work exactly?
There are two ways:
I make and add features and fix issues via pull requests, then you approve them and pull them into master, and then publish to NPM.

Or, I become a contributer on the repo, and I manage it all.

It makes no sense for me to have NPM permission but not GitHub permission. Let me know which one works for you - I don't mind either way, and I can tell you how to use NPM publish properly since I figured it out today.

@dongsik-yoo
Copy link
Contributor

@worthy7 Actually, I have to choose first option that you suggest. Because NHN Entertainment has the company's management policy of their repositories.

I also agree with you about different permission between NPM and Github. But it can't be done with this NHN repository.

How about using another independent repository again?

@worthy7
Copy link

worthy7 commented Aug 8, 2018

So who in NHN is going to check Pull requests and publish to NPM?
If there is nobody then it's entirely pointless.
I can teach you about how to do the NPM publishes. I wrote it here:
https://github.com/Worthy7/tui.ngx-calendar/wiki/How-to-NPM-Publish-(for-devs)

Remove my permission from NPM then. I would rather keep this simple for myself. At some point I think that NHN should be contributing to this angular version anyway, so keeping the repo with you guys seems logical.

@worthy7
Copy link

worthy7 commented Aug 8, 2018

@dongsik-yoo Please merge in my pull request for ReadOnly, then the NPM changes one, and try to publish to NPM yourself then :-)

@nicholasgcoles
Copy link

@dongsik-yoo @worthy7 , is there any update on the Angular 5/6/7 wrapper for this component?

@worthy7
Copy link

worthy7 commented Nov 15, 2018

@nicholasgcoles It's completely dead and it looks like it will stay that way.

@dongsik-yoo
Copy link
Contributor

Typescript interface is here v1.9.1.

Features automation moved this from To do to Done Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Features
  
Done
Development

No branches or pull requests

9 participants