Skip to content

Conversation

@begedin
Copy link
Contributor

@begedin begedin commented Feb 14, 2017

What's in this PR?

This PR adds basic skill assignment UI to the new task page. It works basically like the UI on the edit page, except the task-skill records are actually created after the task is saved.

Styling is not final, but can be dealt with in another PR.

The section listing the project skills for quick adding is also not there. It can be added with #1052

This should be reviewed and merged after reviewing and merging #1045

I also made a code change in the x-skill service layer. I renamed .contains to .includes, reason being .contains is being deprecated on ember arrays. Since for the task new page, we use a plain ember array instead of a service, it would make sense for the interface to match.

References

Progress on: #1046

@begedin begedin self-assigned this Feb 14, 2017
@begedin begedin requested a review from joshsmith February 14, 2017 12:04
@begedin begedin added this to the Assign skills to tasks milestone Feb 14, 2017
@begedin begedin changed the title 1046 skill ui to new task page Add skill ui to new task page Feb 14, 2017
@joshsmith joshsmith self-assigned this Feb 14, 2017
@begedin begedin force-pushed the 1046-skill-ui-to-new-task-page branch from e8a3ff3 to 11f51e3 Compare February 15, 2017 09:40
@joshsmith joshsmith force-pushed the 1046-skill-ui-to-new-task-page branch from 11f51e3 to 38872df Compare February 15, 2017 13:11
.then((task) => this.transitionToRoute('project.tasks.task', get(task, 'number')))
.catch((payload) => this._handleTaskSaveError(payload));
},
deselectSkill(skill) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing here.

let unsavedTaskSkills = get(this, 'unsavedTaskSkills');
let promises = unsavedTaskSkills.map((skill) => this._createTaskSkill(skill, task));
return RSVP.all(promises).then(() => task);
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this below _create... to keep alpha?

},

contains(skill) {
includes(skill) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like renaming to includes here.

@joshsmith joshsmith force-pushed the 1046-skill-ui-to-new-task-page branch from 38872df to c438bfa Compare February 15, 2017 13:22
@joshsmith joshsmith merged commit 16ba87d into develop Feb 15, 2017
@joshsmith joshsmith deleted the 1046-skill-ui-to-new-task-page branch February 15, 2017 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants