Skip to content

Conversation

@begedin
Copy link
Contributor

@begedin begedin commented May 10, 2017

What's in this PR?

This PR adds a github-issue-select component, to be used on a task edit page, to connect an unconnected task with a github issue.

The implementation requires some fixes in #1275 so it branches off of that branch and requires that PR to be merged first.

The implemented behavior is near-identical to that of github-repository-select implemented in #1275.

We need to provide an issues collection, which can be a promise, as well as a task model to the component.

If the task has no githubId, the dropdown will render placeholder text and allow selecting an issue. Upon selecting an issue, the component will send out an action with the selected issue to the parent.

If the task has a githubId, the issue with that github id will be rendered as selected, and the dropdown will be disabled, since we ought to not support re-connecting in the MVP.

References

Fixes #1274
Requires merging of #1275
Requires changing PR base to develop once #1275 is merged.

options=issues
onchange=onIssueSelected
placeholder='Connect your task with a GitHub issue'
renderInPlace=true
Copy link
Contributor

Choose a reason for hiding this comment

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

So we have a power-select/component.js where we define our common defaults....loadingMessage, searchMessage, etc. If all power selects are renderInPlace=true, then this would be a great place to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all of them are. The one for task assignment is causing layout problems if we render in place, due to parent scroll bars.

There is, however, a fix listed in the power-select changelog that may have resolved it. We could look into it, but probably not as part of this issue.

}
});

function renderPage() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to just inline page.render in the beforeEach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually would. I started the implementation thinking I'd be calling renderPage() for each test individually.

export default {
scope: '.github-issue-select',

openDropdown() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it potentially make sense to utilize the pages/components/power-select file and maybe make it more dynamic (ie. pass .github-issue-select)? Looks like somehow the trigger.open in the PS page file could take this.scope from this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look into it, but I think there was a problem with that page object working for a power-select that isn't rendered in place.

@begedin begedin force-pushed the 1274-github-issue-select branch from b939ef2 to 0b88481 Compare August 8, 2017 08:36
@begedin begedin changed the base branch from 1273-add-github-repo-select to develop August 8, 2017 08:37
@begedin begedin changed the base branch from develop to 1273-add-github-repo-select August 8, 2017 08:37
@begedin begedin force-pushed the 1273-add-github-repo-select branch 2 times, most recently from 36db856 to ef90081 Compare August 14, 2017 12:07
@begedin
Copy link
Contributor Author

begedin commented Aug 14, 2017

With the changes in our project - github repo relationships, this PR is no longer relevant.

@begedin begedin closed this Aug 14, 2017
@joshsmith joshsmith deleted the 1274-github-issue-select branch August 17, 2017 22:49
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