Closed kevcjones-archived closed 6 years ago
Tbh I'm with you, once in style block it's incredibly hard to override other than using more JS.
On 29 Nov 2017 18:08, "Celso Trindade" notifications@github.com wrote:
@celsomtrindade commented on this pull request.
In src/simple-modal-holder.component.ts https://github.com/KevCJones/ngx-simple-modal/pull/5#discussion_r153869183 :
- }
- setTimeout(() => {
- modalWrapper.wrapper.nativeElement.classList.add('show');
- modalWrapper.wrapper.nativeElement.classList.add('in');
- });
- if (options.autoCloseTimeout) {
- setTimeout(() => {
- this.removeModal(_component);
- }, options.autoCloseTimeout);
- }
- if (options.closeByClickingOutside) {
- modalWrapper.onClickOutsideModalContent( () => {
- this.removeModal(modalWrapper.content);
- });
- }
- if (options.backdropColor) {
I must reinforce that this is just my personal preference and opinion.
In this case, I'm against any type of style control via options. I think the style belong to the css, where it can be better integrated with the general styleguide of each individual project without the need to change a hex color, for example, in the javascript if it changes in the css. What if I want a background color with transparency of 0.54? Maybe there will have to be another option to use in conjunction with the backdropColor, wich adds unecessary complexity to the code, when it's easily done in plain css.
It's better to maintain where it belongs. I can provide an initial css that is easy to understand and personalize.
I think the options should be related to modal behavior only, such as closing by escape key, click outside, max number of modal open at the same time, etc..
And again, just my preference. If theres is a better argument to why we should keep it in the js, let's do it.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/pull/5#discussion_r153869183, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqe8CjTFrdg_ipO63l8ICkY3-3DY51lks5s7Z2OgaJpZM4QvSvi .
Then I think it's good to go! There is a lot of improvements on this code already!
Resolves #4 Renaming to use a consistent 'modal' / 'simpleModal' name convention. Dialog is an implementation detail. The only dialog reference left in is one for accessibility role='dialog'