Do not show the placeholder text when editing is disabled

bustle · Sep 16, 2015 · 03e404c · 03e404c
1 parent 58b6c11
commit 03e404c
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 24 deletions.
diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js
@@ -18,7 +18,7 @@ import MobiledocRenderer from '../renderers/mobiledoc';
 import { mergeWithOptions } from 'content-kit-utils';
 import { clearChildNodes, addClassName, parseHTML } from '../utils/dom-utils';
 import { forEach, filter } from '../utils/array-utils';
-import { getData, setData } from '../utils/element-utils';
+import { setData } from '../utils/element-utils';
 import mixin from '../utils/mixin';
 import EventListenerMixin from '../utils/event-listener';
 import Cursor from '../utils/cursor';
@@ -138,8 +138,6 @@ class Editor {
     this.element = element;
 
     addClassName(this.element, EDITOR_ELEMENT_CLASS_NAME);
-    this.applyPlaceholder();
-
     element.spellcheck = this.spellcheck;
 
     if (this.isEditable === null) {
@@ -149,7 +147,6 @@ class Editor {
     clearChildNodes(element);
 
     this._setupListeners();
-
     this._addEmbedIntent();
     this._addToolbar();
     this._addTooltip();
@@ -159,9 +156,7 @@ class Editor {
     this.run(() => {});
     this.rerender();
 
-    if (this.autofocus) {
-      element.focus();
-    }
+    if (this.autofocus) { this.element.focus(); }
   }
 
   _addToolbar() {
@@ -300,13 +295,8 @@ class Editor {
     return new Cursor(this);
   }
 
-  applyPlaceholder() {
-    const placeholder = this.placeholder;
-    const existingPlaceholder = getData(this.element, 'placeholder');
-
-    if (placeholder && !existingPlaceholder) {
-      setData(this.element, 'placeholder', placeholder);
-    }
+  setPlaceholder(placeholder) {
+    setData(this.element, 'placeholder', placeholder);
   }
 
   /**
@@ -435,6 +425,7 @@ class Editor {
     this.isEditable = false;
     if (this.element) {
       this.element.setAttribute('contentEditable', false);
+      this.setPlaceholder('');
     }
   }
 
@@ -449,6 +440,7 @@ class Editor {
     this.isEditable = true;
     if (this.element) {
       this.element.setAttribute('contentEditable', true);
+      this.setPlaceholder(this.placeholder);
     }
   }
 

diff --git a/src/js/utils/element-utils.js b/src/js/utils/element-utils.js
@@ -101,15 +101,6 @@ function positionElementToRightOf(element, rightOfElement) {
   return positionElementToRect(element, rightOfElementRect, -verticalCenter, -rightOfElement.offsetWidth - elementMargin);
 }
 
-function getData(element, name) {
-  if (element.dataset) {
-    return element.dataset[name];
-  } else {
-    const dataName = dasherize(name);
-    return element.getAttribute(dataName);
-  }
-}
-
 function setData(element, name, value) {
   if (element.dataset) {
     element.dataset[name] = value;
@@ -120,7 +111,6 @@ function setData(element, name, value) {
 }
 
 export {
-  getData,
   setData,
   createDiv,
   hideElement,

diff --git a/tests/acceptance/basic-editor-test.js b/tests/acceptance/basic-editor-test.js
@@ -41,6 +41,17 @@ test('#disableEditing before render is meaningful', (assert) => {
                'element is contenteditable');
 });
 
+test('when editing is disabled, the placeholder is not shown', (assert) => {
+  editor = new Editor({placeholder: 'the placeholder'});
+  editor.disableEditing();
+  editor.render(editorElement);
+
+  assert.ok(!$('#editor').data('placeholder'), 'no placeholder when disabled');
+  editor.enableEditing();
+  assert.equal($('#editor').data('placeholder'), 'the placeholder',
+               'placeholder is shown when editable');
+});
+
 test('#disableEditing and #enableEditing toggle contenteditable', (assert) => {
   editor = new Editor();
   editor.render(editorElement);