select-box refactoring

- more flexibility (allows custom title)
- less re-render
- more robustness

app/assets/javascripts/discourse/components/category-select-box.js.es6

@@ -17,16 +17,16 @@ export default SelectBoxComponent.extend({
 
   clearable: true,
 
-  filterFunction: function() {
+  filterFunction: function(content) {
     const _matchFunction = (filter, text) => {
       return text.toLowerCase().indexOf(filter) > -1;
     };
 
     return (selectBox) => {
       const filter = selectBox.get("filter").toLowerCase();
-      return _.filter(selectBox.get("content"), (content) => {
-        const category = Category.findById(content[selectBox.get("idKey")]);
-        const text = content[selectBox.get("textKey")];
+      return _.filter(content, (c) => {
+        const category = Category.findById(c[selectBox.get("idKey")]);
+        const text = c[selectBox.get("textKey")];
         if (category && category.get("parentCategory")) {
           const categoryName = category.get("parentCategory.name");
           return _matchFunction(filter, text) || _matchFunction(filter, categoryName);
@@ -56,7 +56,7 @@ export default SelectBoxComponent.extend({
   },
 
   // original method is kept for compatibility
-  selectBoxRowTemplate: function() {
+  templateForRow: function() {
     return (rowComponent) => this.rowContentTemplate(rowComponent.get("content"));
   }.property(),
 

app/assets/javascripts/discourse/components/select-box.js.es6

@@ -25,7 +25,7 @@ export default Ember.Component.extend({
   value: null,
   selectedContent: null,
   noContentText: I18n.t("select_box.no_content"),
-  lastHoveredId: null,
+  lastHovered: null,
 
   idKey: "id",
   textKey: "text",
@@ -48,16 +48,33 @@ export default Ember.Component.extend({
 
   castInteger: false,
 
-  filterFunction: function() {
+  filterFunction: function(content) {
     return (selectBox) => {
       const filter = selectBox.get("filter").toLowerCase();
-      return _.filter(selectBox.get("content"), (content) => {
-        return content[selectBox.get("textKey")].toLowerCase().indexOf(filter) > -1;
+      return _.filter(content, (c) => {
+        return c[selectBox.get("textKey")].toLowerCase().indexOf(filter) > -1;
       });
     };
   },
 
-  selectBoxRowTemplate: function() {
+  titleForRow: function() {
+    return (rowComponent) => {
+      return rowComponent.get(`content.${this.get("textKey")}`);
+    };
+  }.property(),
+
+  shouldHighlightRow: function() {
+    return (rowComponent) => {
+      const id = this._castInteger(rowComponent.get(`content.${this.get("idKey")}`));
+      if (Ember.isNone(this.get("lastHovered"))) {
+        return id === this.get("value");
+      } else {
+        return id === this.get("lastHovered");
+      }
+    };
+  }.property(),
+
+  templateForRow: function() {
     return (rowComponent) => {
       let template = "";
 
@@ -65,7 +82,8 @@ export default Ember.Component.extend({
         template += iconHTML(Handlebars.escapeExpression(rowComponent.get("content.icon")));
       }
 
-      template += `<p class="text">${Handlebars.escapeExpression(rowComponent.get("text"))}</p>`;
+      const text = rowComponent.get(`content.${this.get("textKey")}`);
+      template += `<p class="text">${Handlebars.escapeExpression(text)}</p>`;
 
       return template;
     };
@@ -97,9 +115,8 @@ export default Ember.Component.extend({
   init() {
     this._super();
 
-    if (!this.get("content")) {
-      this.set("content", []);
-    }
+    const content = this.getWithDefault("content", []);
+    this.set("content", content);
 
     if (this.site.isMobileDevice) {
       this.set("filterable", false);
@@ -107,8 +124,7 @@ export default Ember.Component.extend({
 
     this.setProperties({
       value: this._castInteger(this.get("value")),
-      componentId: this.elementId,
-      filteredContent: []
+      componentId: this.elementId
     });
   },
 
@@ -208,74 +224,48 @@ export default Ember.Component.extend({
     });
   },
 
-  @observes("value")
-  _valueChanged: function() {
-    if (Ember.isNone(this.get("value"))) {
-      this.set("lastHoveredId", null);
-    }
-  },
-
   @observes("expanded")
   _expandedChanged: function() {
     if (this.get("expanded")) {
       this.setProperties({ focused: false, renderBody: true });
 
-      if (Ember.isNone(this.get("lastHoveredId"))) {
-        this.set("lastHoveredId", this.get("value"));
-      }
-
       if (this.get("filterable")) {
         Ember.run.schedule("afterRender", () => this.$(".filter-query").focus());
       }
     };
   },
 
-  @computed("value", "content")
+  @computed("value", "content.[]")
   selectedContent(value, content) {
     if (Ember.isNone(value)) {
       return null;
     }
 
-    const selectedContent = content.find((c) => {
-      return c[this.get("idKey")] === value;
+    return content.find((c) => {
+      return this._castInteger(c[this.get("idKey")]) === value;
     });
-
-    if (!Ember.isNone(selectedContent)) {
-      return this._normalizeContent(selectedContent);
-    }
-
-    return null;
   },
 
-  @computed("headerText", "dynamicHeaderText", "selectedContent.text")
-  generatedHeadertext(headerText, dynamic, text) {
-    if (dynamic && !Ember.isNone(text)) {
-      return text;
+  @computed("headerText", "dynamicHeaderText", "selectedContent", "textKey")
+  generatedHeadertext(headerText, dynamic, selectedContent, textKey) {
+    if (dynamic && !Ember.isNone(selectedContent)) {
+      return selectedContent[textKey];
     }
 
     return headerText;
   },
 
-  @observes("content.[]", "filter")
-  _filterChanged: function() {
-    if (Ember.isEmpty(this.get("filter"))) {
-      this.set("filteredContent", this._remapContent(this.get("content")));
-    } else {
-      const filtered = this.filterFunction()(this);
-      this.set("filteredContent", this._remapContent(filtered));
-    }
-  },
+  @computed("content.[]", "filter")
+  filteredContent(content, filter) {
+    let filteredContent;
 
-  @observes("content.[]", "value")
-  @on("didReceiveAttrs")
-  _contentChanged: function() {
-    if (!Ember.isNone(this.get("value"))) {
-      this.set("lastHoveredId", this.get("content")[this.get("idKey")]);
+    if (Ember.isEmpty(filter)) {
+      filteredContent = content;
     } else {
-      this.set("lastHoveredId", null);
+      filteredContent = this.filterFunction(content)(this);
     }
 
-    this.set("filteredContent", this._remapContent(this.get("content")));
+    return filteredContent;
   },
 
   actions: {
@@ -283,38 +273,22 @@ export default Ember.Component.extend({
       this.toggleProperty("expanded");
     },
 
-    onClearSelection() {
-      this.set("value", null);
-    },
-
     onFilterChange(filter) {
       this.set("filter", filter);
     },
 
-    onSelectRow(id) {
+    onSelectRow(content) {
       this.setProperties({
-        value: this._castInteger(id),
+        value: this._castInteger(content[this.get("idKey")]),
         expanded: false
       });
     },
 
-    onHoverRow(id) {
-      this.set("lastHoveredId", id);
+    onHoverRow(content) {
+      this.set("lastHovered", this._castInteger(content[this.get("idKey")]));
     }
   },
 
-  _remapContent(content) {
-    return content.map(c => this._normalizeContent(c));
-  },
-
-  _normalizeContent(content) {
-    return {
-      id: this._castInteger(content[this.get("idKey")]),
-      text: content[this.get("textKey")],
-      icon: content[this.get("iconKey")]
-    };
-  },
-
   _positionSelectBoxWrapper() {
     const headerHeight = this.$(".select-box-header").outerHeight();
 
@@ -325,11 +299,11 @@ export default Ember.Component.extend({
     });
   },
 
-  _castInteger(value) {
-    if (this.get("castInteger") && Ember.isPresent(value)) {
-      return parseInt(value, 10);
+  _castInteger(id) {
+    if (this.get("castInteger") === true && Ember.isPresent(id)) {
+      return parseInt(id, 10);
     }
 
-    return value;
+    return id;
   }
 });

app/assets/javascripts/discourse/components/select-box/select-box-row.js.es6

@@ -1,44 +1,36 @@
-import { on, observes } from 'ember-addons/ember-computed-decorators';
+import computed from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Component.extend({
+  layoutName: "components/select-box/select-box-row",
+
   classNames: "select-box-row",
 
   tagName: "li",
 
-  classNameBindings: ["isHighlighted"],
+  attributeBindings: ["title"],
 
-  attributeBindings: ["text:title"],
+  classNameBindings: ["isHighlighted:is-highlighted"],
 
-  lastHoveredId: null,
+  @computed("titleForRow")
+  title(titleForRow) {
+    return titleForRow(this);
+  },
 
-  @on("init")
-  @observes("content", "lastHoveredId", "selectedId", "selectBoxRowTemplate")
-  _updateTemplate: function() {
-    this.set("isHighlighted", this._isHighlighted());
-    this.set("text", this.get("content.text"));
-    this.set("template", this.get("selectBoxRowTemplate")(this));
+  @computed("templateForRow")
+  template(templateForRow) {
+    return templateForRow(this);
+  },
+
+  @computed("shouldHighlightRow", "lastHovered", "value")
+  isHighlighted(shouldHighlightRow) {
+    return shouldHighlightRow(this);
   },
 
   mouseEnter() {
-    this.sendAction("onHover", this.get("content.id"));
+    this.sendAction("onHover", this.get("content"));
   },
 
   click() {
-    this.sendAction("onSelect", this.get("content.id"));
-  },
-
-  didReceiveAttrs() {
-    this._super();
-
-    this.set("isHighlighted", this._isHighlighted());
-    this.set("text", this.get("content.text"));
-  },
-
-  _isHighlighted() {
-    if(_.isUndefined(this.get("lastHoveredId"))) {
-      return this.get("content.id") === this.get("selectedId");
-    } else {
-      return this.get("content.id") === this.get("lastHoveredId");
-    }
-  },
+    this.sendAction("onSelect", this.get("content"));
+  }
 });

app/assets/javascripts/discourse/templates/components/select-box.hbs

@@ -17,7 +17,7 @@
   icon=icon
   clearable=clearable
   expanded=expanded
-  selectedId=value
+  value=value
 }}
 
 <div class="select-box-body">
@@ -34,12 +34,14 @@
     {{component selectBoxCollectionComponent
       filteredContent=filteredContent
       selectBoxRowComponent=selectBoxRowComponent
-      selectBoxRowTemplate=selectBoxRowTemplate
-      lastHoveredId=lastHoveredId
+      templateForRow=templateForRow
+      shouldHighlightRow=shouldHighlightRow
+      titleForRow=titleForRow
+      lastHovered=lastHovered
       onSelectRow=(action "onSelectRow")
       onHoverRow=(action "onHoverRow")
       noContentText=noContentText
-      selectedId=value
+      value=value
     }}
   {{/if}}
 </div>

app/assets/javascripts/discourse/templates/components/select-box/select-box-collection.hbs

@@ -2,11 +2,13 @@
   {{#each filteredContent as |content|}}
     {{component selectBoxRowComponent
       content=content
-      selectBoxRowTemplate=selectBoxRowTemplate
-      lastHoveredId=lastHoveredId
+      templateForRow=templateForRow
+      titleForRow=titleForRow
+      shouldHighlightRow=shouldHighlightRow
+      lastHovered=lastHovered
       onSelect=onSelectRow
       onHover=onHoverRow
-      selectedId=selectedId
+      value=value
     }}
   {{else}}
     {{#if noContentText}}

test/javascripts/components/select-box-test.js.es6

@@ -239,12 +239,12 @@ componentTest('supports options to limit size', {
 });
 
 componentTest('supports custom row template', {
-  template: '{{select-box content=content selectBoxRowTemplate=selectBoxRowTemplate}}',
+  template: '{{select-box content=content templateForRow=templateForRow}}',
 
   beforeEach() {
     this.set("content", [{ id: 1, text: "robin" }]);
-    this.set("selectBoxRowTemplate", (rowComponent) => {
-      return `<b>${rowComponent.get("text")}</b>`;
+    this.set("templateForRow", (rowComponent) => {
+      return `<b>${rowComponent.get("content.text")}</b>`;
     });
   },
 
@@ -258,9 +258,10 @@ componentTest('supports custom row template', {
 });
 
 componentTest('supports converting select value to integer', {
-  template: '{{select-box value=2 content=content castInteger=true}}',
+  template: '{{select-box value=value content=content castInteger=true}}',
 
   beforeEach() {
+    this.set("value", 2);
     this.set("content", [{ id: "1", text: "robin"}, {id: "2", text: "régis" }]);
   },
 
@@ -270,6 +271,15 @@ componentTest('supports converting select value to integer', {
     andThen(() => {
       assert.equal(find(".select-box-row.is-highlighted .text").text(), "régis");
     });
+
+    andThen(() => {
+      this.set("value", 3);
+      this.set("content", [{ id: "3", text: "jeff" }]);
+    });
+
+    andThen(() => {
+      assert.equal(find(".select-box-row.is-highlighted .text").text(), "jeff", "it works with dynamic content");
+    });
   }
 });
 
@@ -335,3 +345,20 @@ componentTest('clearable selection', {
     });
   }
 });
+
+componentTest('supports custom row title', {
+  template: '{{select-box content=content titleForRow=titleForRow}}',
+
+  beforeEach() {
+    this.set("content", [{ id: 1, text: "robin" }]);
+    this.set("titleForRow", () => "sam" );
+  },
+
+  test(assert) {
+    click(".select-box-header");
+
+    andThen(() => {
+      assert.equal(find(".select-box-row:first").attr("title"), "sam");
+    });
+  }
+});