nhn / tui.calendar

🍞📅A JavaScript calendar that has everything you need.
http://ui.toast.com/tui-calendar
MIT License
12.04k stars 1.3k forks source link

TypeScript and Angular 5 support #82

Closed kartikbb closed 5 years ago

kartikbb commented 6 years ago

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

dongsik-yoo commented 6 years ago

We'll discuss about your question.

MRisto-zz commented 6 years ago

That would be great. This calendar rocks.

dongsik-yoo commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

I'm on it.

dongsik-yoo commented 6 years ago

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 commented 6 years ago

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

dongsik-yoo commented 6 years ago

@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 commented 6 years ago

@dongsik-yoo Thanks for the help.

dongsik-yoo commented 6 years ago

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

amanvishnani commented 6 years ago

@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 commented 6 years ago

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

ghost commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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

icepeng commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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.

dongsik-yoo commented 6 years ago

I introduce your works at the top of this repo's README. Thanks for your cooperation @brnrds and António.

ghost commented 6 years ago

@amanvishnani any news on your end? About ready to hand this over, any feature requests? Right now it exposes the library's events and methods to Angular.

amanvishnani commented 6 years ago

@brnrds Sorry I was busy for past few days so, didn't checked. If you feel like bare minimal version is ready then sure go ahead.

ghost commented 6 years ago

Updated readme with installation and usage instructions: https://github.com/brnrds/ngx-tui-dev Could someone please try and see if these instructions are clear enough to get it to work?

srinek commented 6 years ago

@brnrds @amanvishnani @icepeng thanks for putting this wrapper. I tried the instructions ... Calendar UI came well ... but the popups are not working... So I added tui-calendar.js into scripts section ( not in instructions) ... now I get below error ( still pop ups are not working) ...

Uncaught TypeError: Cannot read property 'browser' of undefined at Object. (tui-calendar.js:3632) at webpack_require (tui-calendar.js:36) at Object. (tui-calendar.js:2978) at webpack_require (tui-calendar.js:36) at Object. (tui-calendar.js:2681) at webpack_require (tui-calendar.js:36) at Object. (tui-calendar.js:111) at webpack_require (tui-calendar.js:36) at Object. (tui-calendar.js:73) at __webpack_require__ (tui-calendar.js:36)

ghost commented 6 years ago

@srinek popups from TUI Calendar aren't implemented in the Angular plugin at the moment, mainly because the envisioned use-case for this plugin for integration within Angular, using components from Angular Material, for example. Right now you can look in here https://github.com/brnrds/ngx-tui-dev/blob/master/projects/ngx-tui-calendar/src/lib/ngx-tui-calendar.component.ts for the inputs, outputs and methods from TUI Calendar that are exposed to Angular.

srinek commented 6 years ago

@brnrds thanks for the reply. I will check the mentioned ngx-tui-calendar.component.ts for details

dongsik-yoo commented 6 years ago

@srinek TOAST UI Calendar has a mandatory dependency tui-code-snippet. Please refer to the link https://github.com/nhnent/tui.calendar#-dependency.

worthy7 commented 6 years ago

Hey all lets at least move the repo over here please, ngx-tui-calendar is a great name and goes well with other ngx packages, I too will contribute as I want to use this in a commercial project.

@dongsik-yoo can you at least make the repo here please, @brnrds and everyone else lets start PR'ing!

worthy7 commented 6 years ago

@dongsik-yoo Would it be very difficult to remake this in typescript?

If we don't have a typescript version, it means having to keep all the DTO's up to date manually, which will be maintenance hell in the future, and will make development slow.

If we do have a typescript version, it means writing a very small wrapper around it, which will take care of most of the code.

This is a very promising project!

dongsik-yoo commented 6 years ago

@Worthy7

About typescript version I understand advantage and disadvantage what you said. TOAST UI family aren't written in typescript, only in javascript. So I guess it's too difficult to rewrite this calendar. But We will discuss about making typescript interface for TOAST UI.

Offering new repository of NHN Entertainment We've been discuss about offering our official named repository. Two subjects.

  1. What benefits give to offering a repository named under the company to community and users? Some wrappers of tui-editor are already maintained by community.
  2. There are many TOAST UI application and components. Do we need to offer all wrapper package repositories?

We'll decide it ASAP, maybe next week.

worthy7 commented 6 years ago

@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!

dongsik-yoo commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

worthy7 commented 6 years ago

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

dongsik-yoo commented 6 years ago

@Worthy7 I merged your PR.

worthy7 commented 6 years ago

Can you also NPM Release? This kinda needs to be a regular thing if possible

On Mon, 9 Jul 2018 at 17:01, 유동식 notifications@github.com wrote:

@Worthy7 https://github.com/Worthy7 I merged your PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nhnent/tui.calendar/issues/82#issuecomment-403393888, or mute the thread https://github.com/notifications/unsubscribe-auth/AEirH5SCvz42MjsYOhPSdearAjj1QKkCks5uEw3PgaJpZM4Tqtx0 .

dongsik-yoo commented 6 years ago

@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 commented 6 years ago

I have no idea how to release packages but I can learn. Thanks

On Mon, 9 Jul 2018 at 17:43, 유동식 notifications@github.com wrote:

@Worthy7 https://github.com/Worthy7 Did you get the npm access permission from @brnrds https://github.com/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 https://github.com/brnrds release the npm package. What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nhnent/tui.calendar/issues/82#issuecomment-403405016, or mute the thread https://github.com/notifications/unsubscribe-auth/AEirHx9twC7BhoCwbgA-VDbfqmSF7ydTks5uExeegaJpZM4Tqtx0 .

vindiezel23 commented 6 years ago

ngx-tui-calendar-error

vindiezel23 commented 6 years ago

Anyone knows why I am getting the errors above?

@Worthy7 @dongsik-yoo

worthy7 commented 6 years ago

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