diff --git a/app/components/skill-list-item-link.js b/app/components/skill-list-item-link.js index 347c4b91f..3e22d5235 100644 --- a/app/components/skill-list-item-link.js +++ b/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); } diff --git a/app/components/skill-list-item.js b/app/components/skill-list-item.js index ecd9f8e43..2875e7a23 100644 --- a/app/components/skill-list-item.js +++ b/app/components/skill-list-item.js @@ -3,24 +3,70 @@ 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'); @@ -28,6 +74,12 @@ export default Component.extend({ 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); } diff --git a/app/components/skills-list.js b/app/components/skills-list.js deleted file mode 100644 index b64e75384..000000000 --- a/app/components/skills-list.js +++ /dev/null @@ -1,22 +0,0 @@ -import Ember from 'ember'; - -const { - Component -} = Ember; - -/** - This component is used as a container for a list of skills. - - ## default usage - - ```Handlebars - {{skills-list skills=projectSkills}} - ``` - - @class skills-list - @module Component - @extends Ember.Component - */ -export default Component.extend({ - classNames: ['skills-list'] -}); diff --git a/app/styles/_shame.scss b/app/styles/_shame.scss index d31db3892..be17ffe7c 100644 --- a/app/styles/_shame.scss +++ b/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; + } +} diff --git a/app/styles/app.scss b/app/styles/app.scss index cbca4a71b..d077c0de7 100644 --- a/app/styles/app.scss +++ b/app/styles/app.scss @@ -130,7 +130,6 @@ // COMPONENTS - SKILLS // @import "components/skill-list-item"; -@import "components/skills-list"; @import "components/skills-typeahead"; // diff --git a/app/styles/components/skill-list-item.scss b/app/styles/components/skill-list-item.scss index 4027d6745..8bd692140 100644 --- a/app/styles/components/skill-list-item.scss +++ b/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; + } } } } diff --git a/app/styles/components/skills-list.scss b/app/styles/components/skills-list.scss deleted file mode 100644 index 0209e55c2..000000000 --- a/app/styles/components/skills-list.scss +++ /dev/null @@ -1,17 +0,0 @@ -.skills-list { - li { - display: inline-block; - margin-right: 5px; - - &.matched { - color: $blue--dark; - font-weight: 700; - } - } -} - -.task-skills-list { - .skill { - margin-bottom: 5px; - } -} diff --git a/app/templates/components/related-skills.hbs b/app/templates/components/related-skills.hbs index 82cea614d..396737def 100644 --- a/app/templates/components/related-skills.hbs +++ b/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}}
{{#if overflowHidden}} show more diff --git a/app/templates/components/skill-list-item.hbs b/app/templates/components/skill-list-item.hbs index 274cb4812..7aaf1d877 100644 --- a/app/templates/components/skill-list-item.hbs +++ b/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}} + {{skill.title}} {{/if}} diff --git a/app/templates/components/skills-list.hbs b/app/templates/components/skills-list.hbs deleted file mode 100644 index d02afc79f..000000000 --- a/app/templates/components/skills-list.hbs +++ /dev/null @@ -1,3 +0,0 @@ -{{skill-list-items - skills=skills -}} diff --git a/app/templates/project/index.hbs b/app/templates/project/index.hbs index 19b2a5319..63a162cdc 100644 --- a/app/templates/project/index.hbs +++ b/app/templates/project/index.hbs @@ -37,7 +37,7 @@ {{#if projectSkills}}

Skills

- {{skills-list skills=projectSkills}} + {{related-skills skills=projectSkills}}
{{/if}} diff --git a/tests/integration/components/skill-list-item-link-test.js b/tests/integration/components/skill-list-item-link-test.js index 0a1eab38d..510938cc1 100644 --- a/tests/integration/components/skill-list-item-link-test.js +++ b/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(); diff --git a/tests/integration/components/skill-list-item-test.js b/tests/integration/components/skill-list-item-test.js index 38f065bcb..f3e239033 100644 --- a/tests/integration/components/skill-list-item-test.js +++ b/tests/integration/components/skill-list-item-test.js @@ -37,7 +37,7 @@ moduleForComponent('skill-list-item', 'Integration | Component | skill list item } }); -test('it renders and sends an action when its hidden', function(assert) { +test('it renders and sends an action when hidden', function(assert) { assert.expect(2); stubService(this, 'session', mockSession); @@ -53,7 +53,7 @@ test('it renders and sends an action when its hidden', function(assert) {
`); - assert.equal(page.skillListItemLink.skillTitle.text, 'Ruby'); + assert.equal(page.skillListItemSpan.text, 'Ruby'); }); test('it renders and sends no action when not hidden', function(assert) { @@ -68,13 +68,19 @@ test('it renders and sends no action when not hidden', function(assert) { this.render(hbs`{{skill-list-item skill=skill action='skillItemHidden'}}`); - assert.equal(page.skillListItemLink.skillTitle.text, 'Ruby'); + assert.equal(page.skillListItemSpan.text, 'Ruby'); }); -test('it renders the login link', function(assert) { +test('it renders a link when clickable and authenticated', function(assert) { assert.expect(1); - this.render(hbs`{{skill-list-item}}`); + stubService(this, 'session', { isAuthenticated: true }); + this.render(hbs`{{skill-list-item isClickable=true}}`); + assert.ok(page.skillListItemLink.isVisible, 'Renders a link'); +}); - stubService(this, 'session', { authenticated: false }); - assert.ok(page.rendersLogin, 'Renders the login link'); +test('it renders no link when clickable and unauthenticated', function(assert) { + assert.expect(1); + stubService(this, 'session', { isAuthenticated: false }); + this.render(hbs`{{skill-list-item isClickable=true}}`); + assert.notOk(page.skillListItemLink.isVisible, 'Renders no link'); }); diff --git a/tests/integration/components/skill-list-items-test.js b/tests/integration/components/skill-list-items-test.js index a0d906af6..31c953b97 100644 --- a/tests/integration/components/skill-list-items-test.js +++ b/tests/integration/components/skill-list-items-test.js @@ -55,12 +55,12 @@ test('it renders the skills sorted by match and then alphabetically', function(a this.render(hbs`{{skill-list-items skills=skills}}`); assert.equal(page.listItemCount, 4, 'Renders the correct number of skills'); - assert.equal(page.listItems(0).skillListItemLink.skillTitle.text, 'HTML'); - assert.ok(page.listItems(0).skillListItemLink.hasMatched); - assert.equal(page.listItems(1).skillListItemLink.skillTitle.text, 'Ember.js'); - assert.notOk(page.listItems(1).skillListItemLink.hasMatched); - assert.equal(page.listItems(2).skillListItemLink.skillTitle.text, 'Rails'); - assert.notOk(page.listItems(2).skillListItemLink.hasMatched); - assert.equal(page.listItems(3).skillListItemLink.skillTitle.text, 'Ruby'); - assert.notOk(page.listItems(3).skillListItemLink.hasMatched); + assert.equal(page.listItems(0).skillListItemSpan.text, 'HTML'); + assert.ok(page.listItems(0).skillListItemSpan.hasMatched); + assert.equal(page.listItems(1).skillListItemSpan.text, 'Ember.js'); + assert.notOk(page.listItems(1).skillListItemSpan.hasMatched); + assert.equal(page.listItems(2).skillListItemSpan.text, 'Rails'); + assert.notOk(page.listItems(2).skillListItemSpan.hasMatched); + assert.equal(page.listItems(3).skillListItemSpan.text, 'Ruby'); + assert.notOk(page.listItems(3).skillListItemSpan.hasMatched); }); diff --git a/tests/integration/components/skills-list-test.js b/tests/integration/components/skills-list-test.js deleted file mode 100644 index ead90b938..000000000 --- a/tests/integration/components/skills-list-test.js +++ /dev/null @@ -1,33 +0,0 @@ -import Ember from 'ember'; -import { moduleForComponent, test } from 'ember-qunit'; -import hbs from 'htmlbars-inline-precompile'; -import PageObject from 'ember-cli-page-object'; -import skillsList from 'code-corps-ember/tests/pages/components/skills-list'; - -const { set } = Ember; - -let page = PageObject.create(skillsList); - -let skills = [ - { - id: 'skill-1', - title: 'Ember.js' - } -]; - -moduleForComponent('skills-list', 'Integration | Component | skills list', { - integration: true, - beforeEach() { - page.setContext(this); - }, - afterEach() { - page.removeContext(); - } -}); - -test('it renders the skills', function(assert) { - assert.expect(1); - set(this, 'skills', skills); - this.render(hbs`{{skills-list skills=skills}}`); - assert.equal(page.skillListItems.listItemCount, 1, 'Renders all skills'); -}); diff --git a/tests/pages/components/skill-list-item.js b/tests/pages/components/skill-list-item.js index 10b4d4569..411f105dc 100644 --- a/tests/pages/components/skill-list-item.js +++ b/tests/pages/components/skill-list-item.js @@ -1,4 +1,4 @@ -import { isVisible } from 'ember-cli-page-object'; +import { hasClass, isVisible } from 'ember-cli-page-object'; import skillListItemLink from './skill-list-item-link'; export default { @@ -6,5 +6,11 @@ export default { skillListItemLink, + skillListItemSpan: { + scope: 'span', + + hasMatched: hasClass('matched') + }, + rendersLogin: isVisible('a[href$=login]') }; diff --git a/tests/pages/components/skills-list.js b/tests/pages/components/skills-list.js deleted file mode 100644 index 62e992139..000000000 --- a/tests/pages/components/skills-list.js +++ /dev/null @@ -1,7 +0,0 @@ -import skillListItems from './skill-list-items'; - -export default { - scope: '.skills-list', - - skillListItems -};