Repository: exadel-inc/esl
Pull request #1831 (2023-07-26 14:11:08)

feat(esl-utils): extended parser/serializer for marker attributes (PROPOSAL)
- Values `'false'` and `'0'` now considered as false value
- `closeOnEsc`, `closeOnOutsideAction`  (of ESLTogleable), and `ignoreEsc` (ESLTrigger) attributes mapper changed to the new serializer/parser

See detailed behavior in unit test cases

Closes: #1828 


Patch:
diff --git a/src/modules/esl-toggleable/core/esl-toggleable.ts b/src/modules/esl-toggleable/core/esl-toggleable.ts
index d22903b9c..e98fd520a 100644
--- a/src/modules/esl-toggleable/core/esl-toggleable.ts
+++ b/src/modules/esl-toggleable/core/esl-toggleable.ts
@@ -1,8 +1,9 @@
 import {ExportNs} from '../../esl-utils/environment/export-ns';
 import {ESC, SYSTEM_KEYS} from '../../esl-utils/dom/keys';
 import {CSSClassUtils} from '../../esl-utils/dom/class';
-import {prop, attr, jsonAttr, boolAttr, listen} from '../../esl-utils/decorators';
+import {prop, attr, jsonAttr, listen} from '../../esl-utils/decorators';
 import {defined, copyDefinedKeys} from '../../esl-utils/misc/object';
+import {parseBoolean, toBooleanAttribute} from '../../esl-utils/misc/format';
 import {sequentialUID} from '../../esl-utils/misc/uid';
 import {DeviceDetector} from '../../esl-utils/environment/device-detector';
 import {DelayedTask} from '../../esl-utils/async/delayed-task';
@@ -102,11 +103,11 @@ export class ESLToggleable extends ESLBaseElement {
   @attr({name: 'close-on'}) public closeTrigger: string;
 
   /** Disallow automatic id creation when it's empty */
-  @boolAttr() public noAutoId: boolean;
+  @attr({parser: parseBoolean, serializer: toBooleanAttribute}) public noAutoId: boolean;
   /** Close the Toggleable on ESC keyboard event */
-  @boolAttr() public closeOnEsc: boolean;
+  @attr({parser: parseBoolean, serializer: toBooleanAttribute}) public closeOnEsc: boolean;
   /** Close the Toggleable on a click/tap outside */
-  @boolAttr() public closeOnOutsideAction: boolean;
+  @attr({parser: parseBoolean, serializer: toBooleanAttribute}) public closeOnOutsideAction: boolean;
 
   /** Initial params to pass to show/hide action on the start */
   @jsonAttr<ESLToggleableActionParams>({defaultValue: {force: true, initiator: 'init'}})
diff --git a/src/modules/esl-trigger/core/esl-trigger.ts b/src/modules/esl-trigger/core/esl-trigger.ts
index 29700d220..2fced7e28 100644
--- a/src/modules/esl-trigger/core/esl-trigger.ts
+++ b/src/modules/esl-trigger/core/esl-trigger.ts
@@ -2,7 +2,7 @@ import {ExportNs} from '../../esl-utils/environment/export-ns';
 import {ESLBaseElement} from '../../esl-base-element/core';
 import {setAttr} from '../../esl-utils/dom/attr';
 import {attr, boolAttr, prop, listen, ready} from '../../esl-utils/decorators';
-import {parseNumber} from '../../esl-utils/misc/format';
+import {parseBoolean, parseNumber, toBooleanAttribute} from '../../esl-utils/misc/format';
 import {CSSClassUtils} from '../../esl-utils/dom/class';
 import {ENTER, SPACE, ESC} from '../../esl-utils/dom/keys';
 import {ESLTraversingQuery} from '../../esl-traversing-query/core';
@@ -68,7 +68,7 @@ export class ESLTrigger extends ESLBaseElement {
   @attr({defaultValue: '0'}) public hoverHideDelay: string;
 
   /** Prevent ESC keyboard event handling for target element hiding */
-  @boolAttr() public ignoreEsc: boolean;
+  @attr({parser: parseBoolean, serializer: toBooleanAttribute}) public ignoreEsc: boolean;
 
   protected _$target: ESLToggleable | null;
 
diff --git a/src/modules/esl-utils/misc/format.ts b/src/modules/esl-utils/misc/format.ts
index 27a5d1a6e..fc5fd715f 100644
--- a/src/modules/esl-utils/misc/format.ts
+++ b/src/modules/esl-utils/misc/format.ts
@@ -22,11 +22,20 @@ export const unwrapParenthesis = (str: string): string => {
   return str.trim().replace(/^\((.*)\)$/, '$1').trim();
 };
 
+/**
+ * Serialize to boolean string (`'true'|'false'`)
+ * Preserve null, undefined and empty string
+ */
+export const toBooleanAttribute = (val: any): string | null => {
+  if (val === null || val === undefined) return val;
+  return String(!!val && val !== 'false' && val !== '0');
+};
+
 /** Parses `null` and `undefined` as an empty string */
 export const parseString = (val: string | null): string => String(val ?? '');
 
 /** Parses string representation of the boolean value */
-export const parseBoolean = (val: string | null): boolean => val !== null && val !== 'false';
+export const parseBoolean = (val: string | null): boolean => val !== null && val !== 'false' && val !== '0';
 
 /**
  * Parses number with the ability to pass an alternative fallback for NaN.


Test patch:
diff --git a/src/modules/esl-utils/decorators/test/attr.extended.test.ts b/src/modules/esl-utils/decorators/test/attr.extended.test.ts
new file mode 100644
index 000000000..088923892
--- /dev/null
+++ b/src/modules/esl-utils/decorators/test/attr.extended.test.ts
@@ -0,0 +1,100 @@
+import '../../../../polyfills/es5-target-shim';
+import {attr} from '../attr';
+import {setAttr} from '../../dom/attr';
+import {parseBoolean, toBooleanAttribute} from '../../misc/format';
+
+describe('Decorator: attr', () => {
+  describe('Decorator: attr - number parser', () => {
+    class TestElement extends HTMLElement {
+      @attr({parser: parseFloat})
+      public attrNumber: number;
+    }
+    customElements.define('test-el-attr-ext-1', TestElement);
+    const $el = new TestElement();
+    customElements.upgrade($el);
+
+    test('Initial value is set', () => expect($el.attrNumber).toBe(NaN));
+
+    test('Setting a null leads to remove of the attribute with a correct parsing state', () => {
+      $el.attrNumber = 1;
+      $el.attrNumber = null as any;
+      expect($el.attrNumber).toBe(NaN);
+      expect($el.hasAttribute('attr-number')).toBe(false);
+    });
+
+    test.each([
+      ['0', 0],
+      ['1', 1],
+      ['2', 2],
+      [' 1 ', 1]
+    ])('Setting a "%s" value leeds to attribute creation', (attrVal: string, value: number) => {
+      setAttr($el, 'attr-number', attrVal);
+      expect($el.attrNumber).toBe(value);
+    });
+  });
+
+  describe('Decorator: attr - boolean parser', () => {
+    class TestElement extends HTMLElement {
+      @attr({parser: parseBoolean, serializer: Boolean})
+      public attrBoolean: boolean;
+    }
+    customElements.define('test-el-attr-ext-2', TestElement);
+    const $el = new TestElement();
+    customElements.upgrade($el);
+
+    test('Initial value is set', () => expect($el.attrBoolean).toBe(false));
+
+    test('Setting a null leads to remove of the attribute with a correct parsing state', () => {
+      $el.attrBoolean = true;
+      $el.attrBoolean = null as any;
+      expect($el.attrBoolean).toBe(false);
+      expect($el.hasAttribute('attr-boolean')).toBe(false);
+    });
+
+    test.each(
+      ['true', '1', '']
+    )('Setting a "%s" value leeds to attribute creation', (value) => {
+      setAttr($el, 'attr-boolean', value);
+      expect($el.attrBoolean).toBe(true);
+    });
+    test.each(
+      ['false', '0', null]
+    )('Setting a "%s" value leeds to attribute removal', (value) => {
+      setAttr($el, 'attr-boolean', value);
+      expect($el.attrBoolean).toBe(false);
+    });
+  });
+
+  describe('Decorator: attr - boolean serializer', () => {
+    class TestElement extends HTMLElement {
+      @attr({parser: parseBoolean, serializer: toBooleanAttribute})
+      public attrBoolean: boolean;
+    }
+    customElements.define('test-el-attr-ext-3', TestElement);
+    const $el = new TestElement();
+    customElements.upgrade($el);
+
+    test.each(
+      [undefined, null]
+    )('Setting a %o value leeds to attribute removal', () => {
+      $el.attrBoolean = undefined as any;
+      expect($el.hasAttribute('attr-boolean')).toBe(false);
+    });
+
+    test.each(
+      [false, 'false', 0, '0', '']
+    )('Setting a %o value consider as FALSY', (val: any) => {
+      $el.attrBoolean = val;
+      expect($el.attrBoolean).toBe(false);
+      expect($el.getAttribute('attr-boolean')).toBe('false');
+    });
+
+    test.each(
+      [true, 'true', 1, '1', ' ']
+    )('Setting a %o value consider as TRUTHY', (val: any) => {
+      $el.attrBoolean = val;
+      expect($el.attrBoolean).toBe(true);
+      expect($el.getAttribute('attr-boolean')).toBe('true');
+    });
+  });
+});
diff --git a/src/modules/esl-utils/decorators/test/attr.test.ts b/src/modules/esl-utils/decorators/test/attr.test.ts
index 15be8e549..f44f2f865 100644
--- a/src/modules/esl-utils/decorators/test/attr.test.ts
+++ b/src/modules/esl-utils/decorators/test/attr.test.ts
@@ -1,6 +1,5 @@
 import '../../../../polyfills/es5-target-shim';
 import {attr} from '../attr';
-import {parseBoolean} from '../../misc/format';
 
 describe('Decorator: attr', () => {
 
@@ -19,11 +18,6 @@ describe('Decorator: attr', () => {
     public readonlyField: string | null;
     @attr({defaultValue: 'def'})
     public defField: string | boolean;
-
-    @attr({parser: parseFloat})
-    public attrNumber: number;
-    @attr({parser: parseBoolean, serializer: Boolean})
-    public attrBoolean: boolean;
   }
   customElements.define('test-el-attr', TestElement);
   const el = new TestElement();
@@ -107,31 +101,6 @@ describe('Decorator: attr', () => {
     expect(el.hasAttribute('def-field')).toBe(false);
   });
 
-  test('Decorator: attr - number parser', () => {
-    expect(el.attrNumber).toBe(NaN);
-    el.attrNumber = 1;
-    expect(el.attrNumber).toBe(1);
-    expect(el.getAttribute('attr-number')).toBe('1');
-    el.attrNumber = 2;
-    expect(el.attrNumber).toBe(2);
-    expect(el.getAttribute('attr-number')).toBe('2');
-    el.attrNumber = null as any;
-    expect(el.attrNumber).toBe(NaN);
-  });
-
-
-  test('Decorator: attr - boolean parser', () => {
-    expect(el.attrBoolean).toBe(false);
-    el.attrBoolean = true;
-    expect(el.attrBoolean).toBe(true);
-    expect(el.hasAttribute('attr-boolean')).toBe(true);
-    el.attrBoolean = false;
-    expect(el.attrBoolean).toBe(false);
-    expect(el.hasAttribute('attr-boolean')).toBe(false);
-    el.attrBoolean = null as any;
-    expect(el.attrBoolean).toBe(false);
-  });
-
   afterAll(() => {
     document.body.removeChild(el);
   });
diff --git a/src/modules/esl-utils/misc/test/format.test.ts b/src/modules/esl-utils/misc/test/format.test.ts
index 77fa80a8b..986e259a4 100644
--- a/src/modules/esl-utils/misc/test/format.test.ts
+++ b/src/modules/esl-utils/misc/test/format.test.ts
@@ -104,7 +104,7 @@ describe('misc/format helper tests', () => {
       ['false', false],
       ['', true],
       ['true', true],
-      ['0', true],
+      ['0', false],
       ['1', true],
       ['brr', true]
     ])(