Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ember-power-select-with-create instead of ember-power-select #6728

Merged
merged 12 commits into from
Jun 3, 2019

Conversation

madalynrose
Copy link
Contributor

@madalynrose madalynrose commented May 13, 2019

Screen Shot 2019-05-14 at 12 24 13 PM

Are we happy with the styling?

@@ -96,7 +96,7 @@
"ember-load-initializers": "^1.1.0",
"ember-maybe-import-regenerator": "^0.1.6",
"ember-maybe-in-element": "^0.1.3",
"ember-power-select": "^2.0.12",
"ember-power-select-with-create": "cibernox/ember-power-select-with-create#6203918f247c1c5d692db4f9185ab8321d2125e1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we lock to a version instead of a sha? Maybe try 0.6.0 if 0.6.1 was a problem. What was the issue with the tagged 0.6.1?

Copy link
Contributor

Choose a reason for hiding this comment

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

Chatted about this and it's because we need placeholderComponent which was added cibernox/ember-power-select-with-create#82 but there's no release with that yet.

Copy link
Contributor

@meirish meirish left a comment

Choose a reason for hiding this comment

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

One question re: versions but looks good otherwise!

@joshuaogle
Copy link
Contributor

joshuaogle commented May 14, 2019

This looks great! I'm glad the UX is already exactly what we had in mind. Can we improve the placeholder to make the add option clearer by changing it from Search to Search or add a new entity ID? If possible we could also add it to the add menu option like Add a new entity with ID "blah"

@meirish
Copy link
Contributor

meirish commented May 14, 2019

It could be a group ID or an entity ID, right? Is there a way to pass in that text somehow?

Copy link
Contributor

@meirish meirish left a comment

Choose a reason for hiding this comment

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

One thing we should add before this goes in - looks great other than that though!

@madalynrose madalynrose requested a review from a team May 20, 2019 21:06
@meirish meirish added this to the 1.1.3 milestone Jun 3, 2019
@@ -84,7 +84,6 @@
@onChange={{action (action "setAndBroadcast" valuePath)}}
@inputValue={{get model valuePath}}
@helpText={{attr.options.helpText}}
@warning={{attr.options.warning}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this get removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh never mind, I see it was removed from the component below!

Copy link
Contributor

@meirish meirish left a comment

Choose a reason for hiding this comment

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

This looks great!

@meirish
Copy link
Contributor

meirish commented Jun 3, 2019

Going to merge this so we can get it into 1.1.3

@meirish meirish merged commit 890cf8a into master Jun 3, 2019
@meirish meirish deleted the search-select-create branch June 3, 2019 20:26
meirish pushed a commit that referenced this pull request Jun 4, 2019
* use ember-power-select-with-create instead of ember-power-select

* add custom Add message to clarify whether you need a name or ID

* add search-select to storybook

* add wormhole div for ember-basic-dropdown

* add search-select to storybook

* make sure knobs are working

* remove unused code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants