feat(ObserverLocator): special handling for src and href

aurelia · Oct 15, 2017 · 1c231ee · 1c231ee
1 parent ee19393
commit 1c231ee
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 21 deletions.
diff --git a/src/element-observation.js b/src/element-observation.js
@@ -25,7 +25,13 @@ export class XLinkAttributeObserver {
 
 export const dataAttributeAccessor = {
   getValue: (obj, propertyName) => obj.getAttribute(propertyName),
-  setValue: (value, obj, propertyName) => obj.setAttribute(propertyName, value)
+  setValue: (value, obj, propertyName) => {
+    if (value === null || value === undefined) {
+      obj.removeAttribute(propertyName);
+    } else {
+      obj.setAttribute(propertyName, value);
+    }
+  }
 };
 
 export class DataAttributeObserver {
@@ -39,6 +45,9 @@ export class DataAttributeObserver {
   }
 
   setValue(newValue) {
+    if (newValue === null || newValue === undefined) {
+      return this.element.removeAttribute(this.propertyName);
+    }
     return this.element.setAttribute(this.propertyName, newValue);
   }
 

diff --git a/src/observer-locator.js b/src/observer-locator.js
@@ -189,7 +189,10 @@ export class ObserverLocator {
         return this.getObserver(obj, propertyName);
       }
       if (/^\w+:|^data-|^aria-/.test(propertyName)
-        || obj instanceof DOM.SVGElement && this.svgAnalyzer.isStandardSvgAttribute(obj.nodeName, propertyName)) {
+        || obj instanceof DOM.SVGElement && this.svgAnalyzer.isStandardSvgAttribute(obj.nodeName, propertyName)
+        || obj.tagName.toLowerCase() === 'img' && propertyName === 'src'
+        || obj.tagName.toLowerCase() === 'a' && propertyName === 'href'
+      ) {
         return dataAttributeAccessor;
       }
     }

diff --git a/test/composite-observer.spec.js b/test/composite-observer.spec.js
@@ -33,18 +33,18 @@ describe('CompositeObserver', () => {
     expect(conditionObserver.hasSubscribers()).toBe(true);
     expect(yesObserver.hasSubscribers()).toBe(true);
     expect(noObserver.hasSubscribers()).toBe(false);
-    expect(el.textContent).toBe(obj.yes);
+    expect(el.id).toBe(obj.yes);
 
     obj.condition = false;
     setTimeout(() => {
       expect(conditionObserver.hasSubscribers()).toBe(true);
       expect(yesObserver.hasSubscribers()).toBe(false);
       expect(noObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe(obj.no);
+      expect(el.id).toBe(obj.no);
       obj.no = 'noooo';
 
       setTimeout(() => {
-        expect(el.textContent).toBe(obj.no);
+        expect(el.id).toBe(obj.no);
         binding.unbind();
         expect(conditionObserver.hasSubscribers()).toBe(false);
         expect(yesObserver.hasSubscribers()).toBe(false);
@@ -64,6 +64,7 @@ describe('CompositeObserver', () => {
 
   it('handles Binary expressions', done => {
     let obj = { a: false, b: false, c: 1 };
+    /**@type {Element} */
     let el = createElement('<div></div>');
     document.body.appendChild(el);
     let binding = getBinding(observerLocator, obj, 'a && b || c', el, 'textContent', bindingMode.toView).binding;
@@ -79,18 +80,18 @@ describe('CompositeObserver', () => {
     expect(aObserver.hasSubscribers()).toBe(true);
     expect(bObserver.hasSubscribers()).toBe(false);
     expect(cObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe(obj.c.toString());
+    expect(el.id).toBe(obj.c.toString());
 
     obj.a = true;
     setTimeout(() => {
       expect(aObserver.hasSubscribers()).toBe(true);
       expect(bObserver.hasSubscribers()).toBe(true);
       expect(cObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe(obj.c.toString());
+      expect(el.id).toBe(obj.c.toString());
       obj.b = true;
 
       setTimeout(() => {
-        expect(el.textContent).toBe(obj.a.toString());
+        expect(el.id).toBe(obj.a.toString());
         expect(aObserver.hasSubscribers()).toBe(true);
         expect(bObserver.hasSubscribers()).toBe(true);
         expect(cObserver.hasSubscribers()).toBe(false);
@@ -116,12 +117,12 @@ describe('CompositeObserver', () => {
 
     binding.bind(createScopeForTest(obj));
     expect(conditionObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe((!obj.condition).toString());
+    expect(el.id).toBe((!obj.condition).toString());
 
     obj.condition = false;
     setTimeout(() => {
       expect(conditionObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe((!obj.condition).toString());
+      expect(el.id).toBe((!obj.condition).toString());
       binding.unbind();
       expect(conditionObserver.hasSubscribers()).toBe(false);
       document.body.removeChild(el);
@@ -146,13 +147,13 @@ describe('CompositeObserver', () => {
     expect(aObserver.hasSubscribers()).toBe(true);
     expect(bObserver.hasSubscribers()).toBe(true);
     //expect(testObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe('ab');
+    expect(el.id).toBe('ab');
     obj.a = 'aa';
     setTimeout(() => {
       expect(aObserver.hasSubscribers()).toBe(true);
       expect(bObserver.hasSubscribers()).toBe(true);
       //expect(testObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe('aab');
+      expect(el.id).toBe('aab');
 
       binding.unbind();
       expect(aObserver.hasSubscribers()).toBe(false);
@@ -181,13 +182,13 @@ describe('CompositeObserver', () => {
     expect(aObserver.hasSubscribers()).toBe(true);
     expect(bObserver.hasSubscribers()).toBe(true);
     //expect(testObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe('ab');
+    expect(el.id).toBe('ab');
     obj.a = 'aa';
     setTimeout(() => {
       expect(aObserver.hasSubscribers()).toBe(true);
       expect(bObserver.hasSubscribers()).toBe(true);
       //expect(testObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe('aab');
+      expect(el.id).toBe('aab');
 
       binding.unbind();
       expect(aObserver.hasSubscribers()).toBe(false);
@@ -216,13 +217,13 @@ describe('CompositeObserver', () => {
     expect(aObserver.hasSubscribers()).toBe(true);
     expect(bObserver.hasSubscribers()).toBe(true);
     expect(testObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe('ab');
+    expect(el.id).toBe('ab');
     obj.a = 'aa';
     setTimeout(() => {
       expect(aObserver.hasSubscribers()).toBe(true);
       expect(bObserver.hasSubscribers()).toBe(true);
       expect(testObserver.hasSubscribers()).toBe(true);
-      expect(el.textContent).toBe('aab');
+      expect(el.id).toBe('aab');
 
       binding.unbind();
       expect(aObserver.hasSubscribers()).toBe(false);
@@ -269,7 +270,7 @@ describe('CompositeObserver', () => {
     expect(xObserver.hasSubscribers()).toBe(true);
     expect(yObserver.hasSubscribers()).toBe(true);
     expect(zObserver.hasSubscribers()).toBe(true);
-    expect(el.textContent).toBe('true');
+    expect(el.id).toBe('true');
     obj.a = 0;
     obj.b = 0;
     setTimeout(() => {
@@ -282,7 +283,7 @@ describe('CompositeObserver', () => {
       expect(xObserver.hasSubscribers()).toBe(false);
       expect(yObserver.hasSubscribers()).toBe(false);
       expect(zObserver.hasSubscribers()).toBe(false);
-      expect(el.textContent).toBe('false');
+      expect(el.id).toBe('false');
       obj.a = true;
       obj.b = true;
       obj.x = null;
@@ -297,7 +298,7 @@ describe('CompositeObserver', () => {
         expect(xObserver.hasSubscribers()).toBe(true);
         expect(yObserver.hasSubscribers()).toBe(false);
         expect(zObserver.hasSubscribers()).toBe(false);
-        expect(el.textContent).toBe('hello world');
+        expect(el.id).toBe('hello world');
 
         binding.unbind();
         expect(objObserver.hasSubscribers()).toBe(false);

diff --git a/test/element-observation.spec.js b/test/element-observation.spec.js
@@ -5,7 +5,8 @@ import {
   ValueAttributeObserver,
   XLinkAttributeObserver,
   DataAttributeObserver,
-  StyleObserver
+  StyleObserver,
+  dataAttributeAccessor
 } from '../src/element-observation';
 import {
   createElement,
@@ -257,4 +258,27 @@ describe('element observation', () => {
       executeSharedPropertyObserverTests(obj, observer, done);
     });
   });
+
+  describe('element accessor', () => {
+    it('sets and remove attribute correctly', () => {
+      let cases = [
+        { tag: '<a></a>', attr: 'href' },
+        { tag: '<img />', attr: 'src' }
+      ];
+      cases.forEach(test => {
+        let el = createElement(test.tag);
+        let accessor = locator.getAccessor(el, test.attr);
+        [null, undefined].forEach(val => {
+          accessor.setValue(val, el, test.attr);
+          expect(el.hasAttributes(test.attr)).toBe(false);
+        });
+
+        accessor = locator.getAccessor(el);
+        [true, false, '', NaN, 'foo', 5].forEach(val => {
+          accessor.setValue(val, el, test.attr);
+          expect(el.getAttribute(test.attr)).toBe(val.toString());
+        });
+      });
+    });
+  });
 });
diff --git a/test/observer-locator.spec.js b/test/observer-locator.spec.js
@@ -6,7 +6,8 @@ import {
   ValueAttributeObserver,
   XLinkAttributeObserver,
   DataAttributeObserver,
-  StyleObserver
+  StyleObserver,
+  dataAttributeAccessor
 } from '../src/element-observation';
 import {SelectValueObserver} from '../src/select-value-observer';
 import {CheckedObserver} from '../src/checked-observer';
@@ -236,4 +237,14 @@ describe('ObserverLocator', () => {
   it('getAccessor returns SetterObserver for input.model', () => {
     expect(locator.getAccessor(document.createElement('input'), 'model') instanceof SetterObserver).toBe(true);
   });
+
+  it('getAccesor returns dataAttributeAccesor for anything else', () => {
+    [
+      { tag: 'a', attr: 'href' },
+      { tag: 'img', attr: 'src' }
+    ].forEach(test => {
+      let el = document.createElement(test.tag);
+      expect(locator.getAccessor(el, test.attr)).toBe(dataAttributeAccessor);
+    });
+  });
 });