tidy up class typings and logic for undefined and null (#740)

dojo · Nov 2, 2017 · 6bbf5a8 · 6bbf5a8
1 parent 8da4523
commit 6bbf5a8
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 30 deletions.
diff --git a/src/interfaces.d.ts b/src/interfaces.d.ts
@@ -81,6 +81,8 @@ export interface Projection {
 	update(updatedDNode: DNode | DNode[]): void;
 }
 
+export type SupportedClassName = string | null | undefined;
+
 export type DeferredVirtualProperties = (inserted: boolean) => VirtualDomProperties;
 
 export interface VirtualDomProperties {
@@ -121,10 +123,9 @@ export interface VirtualDomProperties {
 	 */
 	readonly key?: Object;
 	/**
-	 * An object literal like `{important:true}` which allows css classes, like `important` to be added and removed
-	 * dynamically. Can also take a function, that must return an object literal.
+	 * An array of supported class names to be added to classList on a DOM node
 	 */
-	readonly classes?: string | null | (undefined | null | string)[];
+	readonly classes?: SupportedClassName | SupportedClassName[];
 	/**
 	 * An object literal like `{height:'100px'}` which allows styles to be changed dynamically. All values must be strings.
 	 */

diff --git a/src/mixins/Themed.ts b/src/mixins/Themed.ts
@@ -1,4 +1,4 @@
-import { Constructor, WidgetProperties } from './../interfaces';
+import { Constructor, WidgetProperties, SupportedClassName } from './../interfaces';
 import { Registry } from './../Registry';
 import { Injector } from './../Injector';
 import { inject } from './../decorators/inject';
@@ -38,8 +38,8 @@ export const INJECTED_THEME_KEY = Symbol('theme');
  * Interface for the ThemedMixin
  */
 export interface ThemedMixin<T = ClassNames> {
-	theme(classes: string): string | null;
-	theme(classes: (string | null)[]): (null | string)[];
+	theme(classes: SupportedClassName): SupportedClassName;
+	theme(classes: SupportedClassName[]): SupportedClassName[];
 	properties: ThemedProperties<T>;
 }
 
@@ -124,14 +124,14 @@ export function ThemedMixin<E, T extends Constructor<WidgetBase<ThemedProperties
 		 */
 		private _theme: ClassNames = {};
 
-		public theme(classes: string): null | string;
-		public theme(classes: (string | null)[]): (null | string)[];
-		public theme(classes: (string | null)[] | string): null | string | (null | string)[] {
+		public theme(classes: SupportedClassName): SupportedClassName;
+		public theme(classes: SupportedClassName[]): SupportedClassName[];
+		public theme(classes: SupportedClassName | SupportedClassName[]): SupportedClassName | SupportedClassName[] {
 			if (this._recalculateClasses) {
 				this._recalculateThemeClasses();
 			}
 			if (Array.isArray(classes)) {
-				return this._getThemeClasses(classes);
+				return classes.map((className) => this._getThemeClass(className));
 			}
 			return this._getThemeClass(classes);
 		}
@@ -145,22 +145,11 @@ export function ThemedMixin<E, T extends Constructor<WidgetBase<ThemedProperties
 			this._recalculateClasses = true;
 		}
 
-		/**
-		 * Get theme class object from classNames
-		 */
-		private _getThemeClasses(classNames: (string | null)[]): (string | null)[]  {
-			let themeClasses: (string | null)[] = [];
-			for (let i = 0; i < classNames.length; i++) {
-				const className = classNames[i];
-				if (!className) {
-					continue;
-				}
-				themeClasses.push(this._getThemeClass(className));
+		private _getThemeClass(className: SupportedClassName): SupportedClassName {
+			if (className === undefined || className === null) {
+				return className;
 			}
-			return themeClasses;
-		}
 
-		private _getThemeClass(className: string): string | null {
 			const extraClasses = this.properties.extraClasses || {} as any;
 			const themeClassName = this._baseThemeClassesReverseLookup[className];
 			let resultClassNames: string[] = [];

diff --git a/tests/unit/mixins/Themed.ts b/tests/unit/mixins/Themed.ts
@@ -103,11 +103,11 @@ registerSuite('ThemedMixin', {
 				flaggedClasses = ThemedInstance.theme([class1, class2]);
 				assert.deepEqual(flaggedClasses, [ testTheme1.testPath1.class1, baseThemeClasses1.class2 ]);
 			},
-			'should filter out falsy params passed to classes function'() {
+			'should return null and undefineds unprocessed'() {
 				const ThemedInstance = new TestWidget();
 				const { class1, class2 } = baseThemeClasses1;
-				const flaggedClasses = ThemedInstance.theme([class1, null, class2, null, '']);
-				assert.deepEqual(flaggedClasses,  [ class1, class2 ]);
+				const flaggedClasses = ThemedInstance.theme([ class1, null, class2, undefined ]);
+				assert.deepEqual(flaggedClasses,  [ class1, null, class2, undefined ]);
 				assert.isFalse(consoleStub.called);
 			}
 		},
@@ -123,11 +123,11 @@ registerSuite('ThemedMixin', {
 				const { class1, class2 } = baseThemeClasses1;
 				const ThemedInstance = new TestWidget();
 				ThemedInstance.__setProperties__({ theme: testTheme1 });
-				let themeClasses: (string | null)[] | string | null  = ThemedInstance.theme(class1);
-				assert.deepEqual(themeClasses, testTheme1.testPath1.class1);
+				const themeClass = ThemedInstance.theme(class1);
+				assert.deepEqual(themeClass, testTheme1.testPath1.class1);
 				ThemedInstance.__setProperties__({ theme: testTheme2 });
 
-				themeClasses = ThemedInstance.theme([class1, class2]);
+				const themeClasses = ThemedInstance.theme([class1, class2]);
 				assert.deepEqual(themeClasses, [ testTheme2.testPath1.class1, baseThemeClasses1.class2 ]);
 			}
 		},