Skip to content

Commit 9793773

Browse files
committed
fix(config): Fixed broken configuration merge
Interestingly enough, the Object.assign method - originally used to merge the custom options into the default options into the configuration object itself - only does a shallow merge. Thus, deep objects (which do exist a few times) don't get copied properly. This issue doesn't prevent the configuration completely. However, configuration was only possible by either passing no or all options into the NotifierConfig constructor. This issue is now fixed, custom options now get merged properly. A procedural flow merges each sub-object explicitely, and other types of values are copied / overwritten directly.
1 parent 6c142c8 commit 9793773

File tree

1 file changed

+72
-37
lines changed

1 file changed

+72
-37
lines changed

src/models/notifier-config.model.ts

Lines changed: 72 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -114,47 +114,82 @@ export class NotifierConfig implements NotifierOptions {
114114
* @param {NotifierOptions} [customOptions={}] Custom notifier options, optional
115115
*/
116116
public constructor( customOptions: NotifierOptions = {} ) {
117-
const defaultOptions: any = {
118-
animations: {
119-
enabled: true,
120-
hide: {
121-
easing: 'ease',
122-
offset: 50,
123-
preset: 'fade',
124-
speed: 300
125-
},
126-
overlap: 150,
127-
shift: {
128-
easing: 'ease',
129-
speed: 300
130-
},
131-
show: {
132-
easing: 'ease',
133-
preset: 'slide',
134-
speed: 300
135-
}
117+
118+
// Set default values
119+
this.animations = {
120+
enabled: true,
121+
hide: {
122+
easing: 'ease',
123+
offset: 50,
124+
preset: 'fade',
125+
speed: 300
136126
},
137-
behaviour: {
138-
autoHide: 7000,
139-
onClick: false,
140-
onMouseover: 'pauseAutoHide',
141-
showDismissButton: true,
142-
stacking: 4
127+
overlap: 150,
128+
shift: {
129+
easing: 'ease',
130+
speed: 300
143131
},
144-
position: {
145-
horizontal: {
146-
distance: 12,
147-
position: 'left'
148-
},
149-
vertical: {
150-
distance: 12,
151-
gap: 10,
152-
position: 'bottom'
153-
}
132+
show: {
133+
easing: 'ease',
134+
preset: 'slide',
135+
speed: 300
136+
}
137+
};
138+
this.behaviour = {
139+
autoHide: 7000,
140+
onClick: false,
141+
onMouseover: 'pauseAutoHide',
142+
showDismissButton: true,
143+
stacking: 4
144+
};
145+
this.position = {
146+
horizontal: {
147+
distance: 12,
148+
position: 'left'
154149
},
155-
theme: 'material'
150+
vertical: {
151+
distance: 12,
152+
gap: 10,
153+
position: 'bottom'
154+
}
156155
};
157-
Object.assign( this, defaultOptions, customOptions ); // Merge all options
156+
this.theme = 'material'
157+
158+
// The following merges the custom options into the notifier config, respecting the already set default values
159+
// This linear, more explicit and code-sizy workflow is preferred here over a recursive one (because we know the object structure)
160+
// Technical sidenote: Objects are merged, other types of values simply overwritten / copied
161+
if ( customOptions.theme !== undefined ) {
162+
this.theme = customOptions.theme;
163+
}
164+
if ( customOptions.animations !== undefined ) {
165+
if ( customOptions.animations.enabled !== undefined ) {
166+
this.animations.enabled = customOptions.animations.enabled;
167+
}
168+
if ( customOptions.animations.overlap !== undefined ) {
169+
this.animations.overlap = customOptions.animations.overlap;
170+
}
171+
if ( customOptions.animations.hide !== undefined ) {
172+
Object.assign( this.animations.hide, customOptions.animations.hide );
173+
}
174+
if ( customOptions.animations.shift !== undefined ) {
175+
Object.assign( this.animations.shift, customOptions.animations.shift );
176+
}
177+
if ( customOptions.animations.show !== undefined ) {
178+
Object.assign( this.animations.show, customOptions.animations.show );
179+
}
180+
}
181+
if ( customOptions.behaviour !== undefined ) {
182+
Object.assign( this.behaviour, customOptions.behaviour );
183+
}
184+
if ( customOptions.position !== undefined ) {
185+
if ( customOptions.position.horizontal !== undefined ) {
186+
Object.assign( this.position.horizontal, customOptions.position.horizontal );
187+
}
188+
if ( customOptions.position.vertical !== undefined ) {
189+
Object.assign( this.position.vertical, customOptions.position.vertical );
190+
}
191+
}
192+
158193
}
159194

160195
}

0 commit comments

Comments
 (0)