ng2-image-upload
ng2-image-upload copied to clipboard
Final API
As the project has not yet hit 1.0.0 release I think we should take the opportunity to apply best practices to the API, I have the following suggestions:
Outputs should not start with on
as per the angular style guide.
onFileUploadFinish
changes to imageUploaded
or imageUploadFinished
onRemove
changes to imageRemoved
isPending
changes to uploadStateChanged
?
I also think having our own custom Header interface is a waste of time and propose the header input property should just be a map instead:
[headers]="[{ Authorization: 'value' }]"
This seems like a more natural implementation and also directly maps to Angular's Headers
class:
export declare class Headers {
constructor(headers?: Headers | {
[name: string]: any;
} | null);
}
After 1.0.0 we should also endeavour to adhere to semantic versioning.
Fully support this.
@UncleDave, this sounds good. I can start implementing soon.
Bear in mind to be compatible with semantic versioning these API changes would have to be v1.0.0 and not before, so it may be worth spinning up a v1.0.0 branch?
Okay got it. @aberezkin We better have develop branch to make all PRs for v1.0.0. and then this can be merged to master later on.
@hardikpthv, there is one already.
The idea of the development branch is that it is persistent and contains code for the "next" version, essentially. While master is always the latest release and ideally should never be committed to except for the changelog and version number.
I believe here our development branch would be 0.6.5 -> 0.7.0 etc, while we have a parallel branch for 1.0.0 which contains things like the API change. Once 1.0.0 is ready to go it gets merged into development and the usual process continues.
I can make development branch the main branch of the repo so if you mention an issue in PR it gets resolved automatically.
upd: Did it already
@aberezkin, @UncleDave implemented discussed changes.
I have a suggestion for the interface. We can use ng-content
(like they do in the angular team) to provide the component with custom captions to make component usage cleaner and more intuitive.
But I didn't quite figure out how to use default messages if there was no needful content in the component's body. Also, in this case, a user can put not only string but some HTML in the message, not sure if this is even a problem.
Also, I had thoughts about wrapping this lib in AbstractControl
or something like that. Just thoughts, though.
I think the inputs work okay for the messages, ng-content
seems a bit less intuitive just because it's not used in nearly as many places as inputs. Maybe if we want to allow the user to pass in some custom HTML to display, but I don't see anyone asking for that.
AbstractControl
is for validation? Unless we implement a min
property I don't see us validating anything.