search

kevcjones / ngx-simple-modal

A simple unopinionated framework to implement simple modal based behaviour in angular (v2+) projects.
MIT License
52 stars 28 forks source link

Refactor and document. #3

Closed kevcjones-archived closed 6 years ago

kevcjones-archived commented 6 years ago

From @KevCJones on November 13, 2017 9:47

A quick code review has found some obvious undocumented TODO's e.g. DialogOptions having a file but the exported interface still lives inside dialog.service.ts.

The comments are sometimes a bit redundant, i'd like to generate some documentation, which will feed from the comments.

This should help me get a firmer grasp on the code base so i can take on community issues and requests. I will pull from the original REPO for issues when the ones here have been handled

Copied from original issue: KevCJones/angularx-bootstrap-modal#4

kevcjones-archived commented 6 years ago

From @celsomtrindade on November 17, 2017 10:25

One note on this topic is normalize the name accros the project. For example, the module is called BootstrapModal, then the service is called DialogService. There are classes named modal and others named dialog.

There is also the fact that it's called "Bootstrap" and has a lot of references to the bootstrap project, but it works perfectly without any Bootstrap code at all. Maybe it's the case to remove any reference from Bootstrap, including it's own css styles (I can provide it) and make it depend only on Angular.

Note: On the Boostrap topic, do you think it's better to open another issue?

kevcjones-archived commented 6 years ago

Well you raise some good points. Initially i think my goal would be to do as much as i can without any breaking changes. Ultimately we're talking about a module in npm with bootstrap in the title already though, so it probably a version 2.0.0.

To your note : Its probably right to open another issue, and peg it as a version 2.0.0 for now. If the autho of the original one doesn't come back by lets say 3 months from now, i'll ramp up this and start considering this its own thing (with credits in tact of course).

kevcjones-archived commented 6 years ago

There is a good conversation which birthed this repo found here https://github.com/KevCJones/angularx-bootstrap-modal/pull/10

kevcjones-archived commented 6 years ago

This is being done / done. Brough over for historical purposes. #4 takes over from this.