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

Add option to show new creatable item as first or last option #1436

Merged
merged 1 commit into from
Jul 4, 2018

Conversation

Stenerson
Copy link
Contributor

With this change, the new Creatable option will:

  • Default to being the first option in the list (as it is currently).
  • Optionally be displayed as the last item in the list (when set to false)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.337% when pulling 9c963d0 on Stenerson:master into ef3a468 on JedWatson:master.

@RKushnir
Copy link

Seems to be a duplicate of #1416

@Stenerson
Copy link
Contributor Author

Ahh, you're right @RKushnir! 😞

But this one includes a passing test and doesn't include the build files as specified in CONTRIBUTING.md 😉

@blacktemplar
Copy link
Contributor

blacktemplar commented Apr 4, 2017

If I apply your pull request I get the create item at the last position but if I try to switch options with the up and down keys I cannot select this last option. If I do the same on the master branch I can select the first option with the keyboard.

Also if I move with the mouse over the last option it gets not highlighted, instead the first option gets highlighted. That hints to me that somewhere else in the code it is assumed that the first option is the create item.

How can we fix this?

@Stenerson
Copy link
Contributor Author

Hi @blacktemplar,
I just rebased on the latest master, tried my changes on the Createable example and it worked as expected. Both with keyboard and mouse. Tried with Chrome 56 and Firefox 52 on Mac. Any ideas?

@Stenerson
Copy link
Contributor Author

FYI - that commit was just rebasing and fixing a conflict that was introduced to the README file

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.734% when pulling ec4b4fc on Stenerson:master into a0e5855 on JedWatson:master.

@blacktemplar
Copy link
Contributor

I just rechecked my code and I think it was my mistake after merging everything with the newest version here I got it working.

@Abazhenov
Copy link

Are there plans to merge this in anytime soon?

@antholord
Copy link

I'd love to use this feature but I can't get it to work.

Using Virtualized Select on top of it,

<VirtualizedSelect
                                {...domOnlyProps(name)}
                                options={dataItems.items}
                                onChange={name.onChange}
                                value={name.value}
                                name="name"
                                placeholder="Name"
                                onBlur={() => name.onBlur(name.value)}
                                simpleValue={true}
                                multi={true}
                                clearable={false}
                                onBlurResetsInput={false}
                                onCloseResetsInput={false}
                                delimiter="|"
                                selectComponent={Creatable}
                                promptTextCreator={(label) => {return "Search for " + label}}
                                showNewOptionAtTop={false}
                            />

The prop promptTextCreator is also a Creatable prop and it works just fine. See anything wrong?

@shift-keshav-pudaruth
Copy link

It is counter-intuitive to show the 'create option' at the start, to the user when it is already available in the list below. Hence, +1 support for this feature.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.892% when pulling 0bf710c on Stenerson:master into 549d20a on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.892% when pulling b22795d on Stenerson:master into 549d20a on JedWatson:master.

@agirton
Copy link
Collaborator

agirton commented May 17, 2017

@Stenerson I think this is a big UX win. Thank you for the tests also! I'm not sure yet if this should go in 1.0 RC or should wait till after. I'm leaning towards waiting till after 1.0 is released.

@AaronHarris
Copy link

Any chance this will be pushed soon?

@agirton
Copy link
Collaborator

agirton commented Jun 7, 2017

@AaronHarris this will be pulled in after 1.0 release. Will be working with Jed to confirm when that is.

@thierrydelbart
Copy link

+1 support for this feature

@sdafni
Copy link

sdafni commented Oct 25, 2017

looking forward to this

@baweinbe
Copy link

Is there an update on this?

@Stenerson
Copy link
Contributor Author

When v1.0.0 ships I'll resolve the remaining conflicts with master and it will be available to merge into 1.1.0.

🤞

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 93.382% when pulling 9459a04 on Stenerson:master into 0d0ccd3 on JedWatson:master.

@Stenerson
Copy link
Contributor Author

This has been rebased on v1.1.0 and should be ready to merge! 🎉

I also updated the Creatable example to demonstrate this functionality.
image

@Stenerson
Copy link
Contributor Author

@JedWatson, any idea when we can start merging "After 1.0 release items"? My users are still struggling a bit with this, adding a bunch of unwanted items because they're used to typing part of a selection and pressing enter. Moving this to the bottom will be a big help. Thanks! 🙏

@mikepro4
Copy link

mikepro4 commented Jan 29, 2018

Please merge this as soon as possible, really need to use this option. 👍

@hollandmatt
Copy link

Also really looking forward to this one! It will make a huge difference for us.

@Stenerson
Copy link
Contributor Author

@JedWatson, this is an easy win with quite a bit of support. Any thoughts on getting it merged and released soon?

I'm loving all of the effort and excitement around v2 but 10s of thousands of us are still relying on v1 and will be for the foreseeable future. This PR has been ✅with passing tests for over 14 months now.

I seriously respect you for the time and mental burden that a project this popular must take to maintain. I'm definitely not saying this to be negative. I just want to see react-select continue to move forward and part of that is improving the version that people are using in production.

🙏 Thank you for your consideration and your continued effort and dedication to this incredibly important component. 🙏

cc: @agirton @bvaughn

@jossmac jossmac merged commit a3a977d into JedWatson:master Jul 4, 2018
@VdeVentura
Copy link

@Stenerson I love you. TY @jossmac for merging this. I'm planning to migrate to V2 but it it not a top priority right now and the previous behaviour is a bad UX IMO.

@VdeVentura
Copy link

BTW, any chances of updating the https://v1.react-select.com/ to reflect this changes ?

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.

None yet