Refactor skill lists

app/components/skill-list-item-link.js

@@ -8,12 +8,12 @@ const {
 } = Ember;
 
 /**
-  This component is used to add or remove a user's skill when authenticated
-  and show the state of the user's skill, or login when unauthenticated.
+  Shows whether a user has a skill and allows them to add or remove the skill
+  in the context of a list of skills.
 
   ## default usage
 
-  ```Handlebars
+  ```handlebars
   {{skill-list-item-link
     matched=matched
     skill=skill
@@ -30,34 +30,57 @@ export default Component.extend({
   classNameBindings: ['justClicked', 'justRemoved', 'matched'],
   tagName: 'a',
 
+  /**
+   * Whether the user just clicked the skill. Resets to `false` on `mouseLeave`.
+   * @type {Boolean}
+   */
+  justClicked: false,
+
+  /**
+   * Whether the user just removed the skill. Resets to `false` on `mouseLeave`.
+   * @type {Boolean}
+   */
+  justRemoved: false,
+
   session: service(),
 
+  /**
+   * Toggles the `justClicked` and potentially the `justRemoved` states,
+   * and also toggles the skill (add or remove the given skill for the user)
+   *
+   * Prevents the click from bubbling.
+   *
+   * @method click
+   */
   click(e) {
     e.stopPropagation();
 
-    if (get(this, 'session.isAuthenticated')) {
-      this._toggleClickState();
+    this._toggleClickState();
 
-      let skill = get(this, 'skill');
-      get(this, 'toggleSkill')(skill);
-    }
+    let skill = get(this, 'skill');
+    get(this, 'toggleSkill')(skill);
   },
 
+  /**
+   * Resets the `justClicked` and `justRemoved` states when the mouse leaves
+   *
+   * @method mouseLeave
+   */
   mouseLeave() {
-    this._clearClickState();
+    this._resetClickState();
   },
 
-  _clearClickState() {
+  _resetClickState() {
     set(this, 'justClicked', false);
     set(this, 'justRemoved', false);
   },
 
   _toggleClickState() {
-    let matched = get(this, 'matched');
-    if (matched) {
-      set(this, 'justRemoved', true);
+    let userHadSkill = get(this, 'matched');
+    if (userHadSkill) {
+      set(this, 'justRemoved', true); // User just removed an existing skill
     } else {
-      set(this, 'justRemoved', false);
+      set(this, 'justRemoved', false); // User just added a new skill
     }
     set(this, 'justClicked', true);
   }

app/components/skill-list-item.js

@@ -3,31 +3,83 @@ import Ember from 'ember';
 const {
   Component,
   computed,
-  computed: { alias, notEmpty },
+  computed: { alias, and, notEmpty },
   get,
   inject: { service }
 } = Ember;
 
+/**
+  Shows whether a user has a skill and determines whether to display the
+  `skill-list-item-link` component, depending on the value of `isClickable`
+  and `session.isAuthenticated`.
+
+  ## advanced usage
+
+  ```handlebars
+  {{skill-list-item
+    action="skillItemHidden"
+    isClickable=true
+    skill=skill
+  }}
+  ```
+
+  @class skill-list-item
+  @module Component
+  @extends Ember.Component
+ */
 export default Component.extend({
   classNames: ['skill-list-item'],
   tagName: 'li',
 
-  justClicked: false,
+  /**
+   * Set by the user. Determines whether the user can add/remove skills in
+   * place.
+   *
+   * Consider the UX when determining whether to set this to `true`, e.g.
+   * adding in place works well when the user is joining a project, but
+   * may not make sense in an environment where they're not actively editing
+   * data.
+   *
+   * @type {Boolean}
+   */
+  isClickable: false,
 
   session: service(),
   store: service(),
   userSkillsList: service(),
 
+  /**
+   * Determines whether the user can add/remove the skill by clicking.
+   * @type {Boolean}
+   */
+  canClick: and('isClickable', 'session.isAuthenticated'),
+
+  /**
+   * The `skill` is `matched` if the user has a `userSkill` for that `skill`
+   * @type {Boolean}
+   */
   matched: notEmpty('userSkill'),
+
   user: alias('currentUser.user'),
 
+  /**
+   * Returns the `user`'s `userSkill` for the `skill` by searching their
+   * `userSkills`
+   * @type DS.Model
+   */
   userSkill: computed('skill', 'userSkillsList.userSkills.@each.userSkill', 'userSkillsList.userSkills.isFulfilled', function() {
     let skill = get(this, 'skill');
     let userSkillsList = get(this, 'userSkillsList');
     let result = userSkillsList.find(skill);
     return result;
   }),
 
+  /**
+   * Sends the `action` when the skill list is partially hidden. Useful for the
+   * project card where we want to toggle to expand more than a few lines of
+   * skills.
+   * @method didRender
+   */
   didRender() {
     this._super(...arguments);
     let parentBottom = this.$().parent()[0].getBoundingClientRect().bottom;
@@ -38,6 +90,12 @@ export default Component.extend({
     }
   },
 
+  /**
+   * Adds or removes the skill, depending on whether the user has the skill.
+   * @method toggleSkill
+   * @param {DS.Model} skill
+   * @return {DS.Model} skill
+   */
   toggleSkill(skill) {
     return get(this, 'userSkillsList').toggle(skill);
   }

app/styles/_shame.scss

@@ -112,3 +112,10 @@ h4 span {
     width: 12px;
   }
 }
+
+// Have nowhere logical for this to go now
+.task-skills-list {
+  .skill {
+    margin-bottom: 5px;
+  }
+}

app/styles/app.scss

@@ -130,7 +130,6 @@
 // COMPONENTS - SKILLS
 //
 @import "components/skill-list-item";
-@import "components/skills-list";
 @import "components/skills-typeahead";
 
 //

app/styles/components/skill-list-item.scss

@@ -3,52 +3,55 @@
   padding: 3px 6px;
 }
 
-.skill-list-item {
-  a {
-    color: $text--dark;
-    cursor: pointer;
-    position: relative;
-
-    &:before {
-      content: "";
-      display: inline-block;
-      margin: -2px 2px 0 0;
-      vertical-align: middle;
-    }
-
-    &.just-removed {
-      color: $blue--dark;
-    }
+// Styles both clickable and unclickable items
+.skill-list-item a, .skill-list-item > span {
+  color: $text--dark;
+  position: relative;
+
+  &:before {
+    content: "";
+    display: inline-block;
+    margin: -2px 2px 0 0;
+    vertical-align: middle;
+  }
 
-    &.matched {
-      color: $blue--dark;
+  &.matched {
+    color: $blue--dark;
 
-      &:not(.just-removed) {
-        font-weight: 600;
-        &:before {
-          @include sprite($tiny-check);
-        }
+    &:not(.just-removed) {
+      font-weight: 600;
+      &:before {
+        @include sprite($tiny-check);
       }
     }
+  }
+}
 
-    &:hover:not(.matched) {
-      color: $blue--dark;
-    }
+// Only for the link
+.skill-list-item a {
+  cursor: pointer;
 
-    &:hover.matched {
-      &:not(.just-clicked) {
-        color: $danger-color;
-        text-decoration: line-through;
+  &.just-removed {
+    color: $blue--dark;
+  }
 
-        &:before {
-          @include sprite($tiny-x);
-          margin-left: 2px;
-        }
-      }
-    }
+  &:focus {
+    outline: none;
+  }
 
-    &:focus {
-      outline: none;
+  &:hover:not(.matched) {
+    color: $blue--dark;
+  }
+
+  &:hover.matched {
+    &:not(.just-clicked) {
+      color: $danger-color;
+      text-decoration: line-through;
+
+      &:before {
+        @include sprite($tiny-x);
+        margin-left: 2px;
+      }
     }
   }
 }

app/templates/components/related-skills.hbs

@@ -1,4 +1,4 @@
-{{skill-list-items skills=skills skillItemHidden="skillItemHidden" overflowHidden=overflowHidden}}
+{{skill-list-items overflowHidden=overflowHidden skillItemHidden="skillItemHidden" skills=skills}}
 <div class="expander {{if showToggle 'visible' 'hidden'}}">
   {{#if overflowHidden}}
     <a {{action "showMore"}}>show more</a>

app/templates/components/skill-list-item.hbs

@@ -1,5 +1,5 @@
-{{#if session.isAuthenticated}}
+{{#if canClick}}
   {{skill-list-item-link matched=matched skill=skill toggleSkill=(action toggleSkill)}}
 {{else}}
-  {{#link-to 'login'}}{{skill.title}}{{/link-to}}
+  <span class="{{if matched "matched"}}">{{skill.title}}</span>
 {{/if}}

app/templates/project/index.hbs

@@ -37,7 +37,7 @@
       {{#if projectSkills}}
         <section class="project-sidebar__section">
           <h4>Skills</h4>
-          {{skills-list skills=projectSkills}}
+          {{related-skills skills=projectSkills}}
         </section>
       {{/if}}
 

tests/integration/components/skill-list-item-link-test.js

@@ -62,7 +62,7 @@ test('it renders correctly when unmatched', function(assert) {
 test('it toggles the action when clicked and authenticated', function(assert) {
   assert.expect(1);
 
-  stubService(this, 'session', { isAuthenticated: true });
+  stubService(this, 'session', mockSession);
 
   let skill = { title: 'Ember.js' };
   set(this, 'skill', skill);
@@ -76,23 +76,6 @@ test('it toggles the action when clicked and authenticated', function(assert) {
   page.click();
 });
 
-test('it does not toggle the action when clicked and unauthenticated', function(assert) {
-  assert.expect(0);
-
-  stubService(this, 'session', { isAuthenticated: false });
-
-  let skill = { title: 'Ember.js' };
-  set(this, 'skill', skill);
-
-  function toggleHandler(toggledSkill) {
-    assert.deepEqual(skill, toggledSkill);
-  }
-  setHandlers(this, toggleHandler);
-
-  renderPage();
-  page.click();
-});
-
 test('it does not have clicked or removed classes at first', function(assert) {
   assert.expect(2);
   renderPage();