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

Open
kartikbb opened this Issue Apr 27, 2018 · 63 comments

Comments

Projects
10 participants
@kartikbb

kartikbb commented Apr 27, 2018

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

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented Apr 29, 2018

We'll discuss about your question.

@dongsik-yoo dongsik-yoo added question and removed enhancement labels Apr 29, 2018

@MRisto

This comment has been minimized.

MRisto commented May 3, 2018

That would be great. This calendar rocks.

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented May 3, 2018

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

This comment has been minimized.

amanvishnani commented May 6, 2018

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

This comment has been minimized.

Contributor

dongsik-yoo commented May 8, 2018

I'm on it.

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented May 8, 2018

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

This comment has been minimized.

amanvishnani commented May 8, 2018

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

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented May 8, 2018

@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

This comment has been minimized.

amanvishnani commented May 8, 2018

@dongsik-yoo Thanks for the help.

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented May 8, 2018

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

@amanvishnani

This comment has been minimized.

amanvishnani commented May 8, 2018

@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

This comment has been minimized.

Contributor

dongsik-yoo commented May 9, 2018

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

@brnrds

This comment has been minimized.

brnrds 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

This comment has been minimized.

amanvishnani commented May 9, 2018

@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

This comment has been minimized.

Contributor

dongsik-yoo commented May 9, 2018

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

@brnrds

This comment has been minimized.

brnrds 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

This comment has been minimized.

Contributor

dongsik-yoo commented May 10, 2018

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

@icepeng

This comment has been minimized.

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

This comment has been minimized.

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.

View.prototype.getViewBound = function() {
var container = this.container,
position = domutil.getPosition(container),
size = domutil.getSize(container);
return {
x: position[0],
y: position[1],
width: size[0],
height: size[1]
};
};

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

This comment has been minimized.

amanvishnani commented May 10, 2018

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

This comment has been minimized.

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

This comment has been minimized.

brnrds 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

This comment has been minimized.

amanvishnani commented May 10, 2018

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

This comment has been minimized.

Contributor

dongsik-yoo commented May 11, 2018

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

@brnrds

This comment has been minimized.

brnrds 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.

@brnrds

This comment has been minimized.

brnrds 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

This comment has been minimized.

Worthy7 commented Jun 22, 2018

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

This comment has been minimized.

Worthy7 commented Jun 22, 2018

@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

This comment has been minimized.

Contributor

dongsik-yoo commented Jun 22, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Contributor

dongsik-yoo commented Jul 4, 2018

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

dongsik-yoo commented Jul 9, 2018

@Worthy7 I merged your PR.

@Worthy7

This comment has been minimized.

Worthy7 commented Jul 9, 2018

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented Jul 9, 2018

@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

This comment has been minimized.

Worthy7 commented Jul 9, 2018

@vindiezel23

This comment has been minimized.

vindiezel23 commented Jul 20, 2018

ngx-tui-calendar-error

@vindiezel23

This comment has been minimized.

vindiezel23 commented Jul 20, 2018

Anyone knows why I am getting the errors above?

@Worthy7 @dongsik-yoo

@Worthy7

This comment has been minimized.

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

This comment has been minimized.

Contributor

dongsik-yoo commented Jul 20, 2018

@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

This comment has been minimized.

Worthy7 commented Jul 20, 2018

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

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented Jul 20, 2018

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

@Worthy7

This comment has been minimized.

Worthy7 commented Aug 1, 2018

@brnrds poke

@Worthy7

This comment has been minimized.

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

This comment has been minimized.

@dongsik-yoo

This comment has been minimized.

Contributor

dongsik-yoo commented Aug 6, 2018

Done. @Worthy7 has been added as a maintainer.

@Worthy7

This comment has been minimized.

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

This comment has been minimized.

Contributor

dongsik-yoo commented Aug 8, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

nicholasgcoles commented Nov 13, 2018

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

@Worthy7

This comment has been minimized.

Worthy7 commented Nov 15, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment