-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
@@ -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", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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!
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 |
It could be a group ID or an entity ID, right? Is there a way to pass in that text somehow? |
There was a problem hiding this 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!
…ult into search-select-create
…o search-select-create
@@ -84,7 +84,6 @@ | |||
@onChange={{action (action "setAndBroadcast" valuePath)}} | |||
@inputValue={{get model valuePath}} | |||
@helpText={{attr.options.helpText}} | |||
@warning={{attr.options.warning}} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Going to merge this so we can get it into 1.1.3 |
* 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
Are we happy with the styling?