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

#658 Allow Create functionality [WIP] #660

Closed
wants to merge 3 commits into from

Conversation

vickyonit
Copy link

Fix for following issues:

#658
#586

Tech Changes:

  • Modifies Option to render the "Add " label field if option is create
  • Modifies renderMenu to show the new Option if allowCreate in the list
  • Modifies Select Test Cases - Removed redundant tests

Assumptions:

  • addLabelText contains {label} in it.
  • newOptionCreator always contains the following structure
    { value: value, label: value, create: true }

@hsrobflavorus
Copy link

Thanks, I'm testing this out... the newOptionCreator missing is a big problem for us.

<Select
allowCreate={this.props.allowCreate}
value={this.state.value}
multi={true}
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally preferred that boolean props that are true don't have an explicit declaration. remove the ={true} e.g. https://github.com/JedWatson/react-select/blob/master/examples/src/components/NumericSelect.js#L66

@omgaz
Copy link
Contributor

omgaz commented Dec 18, 2015

Great progress :)

@FoxxMD
Copy link

FoxxMD commented Dec 30, 2015

Looking forward to this!

@miro
Copy link

miro commented Dec 30, 2015

^ mandatory +1

@looshi
Copy link

looshi commented Dec 30, 2015

+1

1 similar comment
@feychenie
Copy link

👍

@RobAWilkinson
Copy link

👍 this no allowCreate is a huge problem for us

@mitani
Copy link

mitani commented Jan 6, 2016

This was a show stopper for us, but there is a workaround. For those of you that need allowCreate today - you can just use filterOptions and add an option {label: Add ${term}, value: term, create:true} and then you just process it in your onChange function where you look for create === true

@donbonifacio
Copy link

👍

omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 8, 2016
 - Custom style overrides
 - single and multi examples
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components
 - `allowCreate` dependant on JedWatson/react-select#660
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 8, 2016
 - Custom style overrides
 - single and multi examples
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components
 - `allowCreate` dependant on JedWatson/react-select#660
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 8, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components
 - `allowCreate` dependant on JedWatson/react-select#660
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 8, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components
 - `allowCreate` dependant on JedWatson/react-select#660
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 11, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components and removed unnecessary `<div>`s
 - `allowCreate` dependant on JedWatson/react-select#660
 - Removed unnecessary box shadow length properties
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 11, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components and removed unnecessary `<div>`s
 - `allowCreate` dependant on JedWatson/react-select#660
 - Removed unnecessary box shadow length properties
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 11, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components and removed unnecessary `<div>`s
 - `allowCreate` dependant on JedWatson/react-select#660
 - Removed unnecessary box shadow length properties
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 12, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components and removed unnecessary `<div>`s
 - `allowCreate` dependant on JedWatson/react-select#660
 - Removed unnecessary box shadow length properties
omgaz added a commit to Adslot/adslot-ui that referenced this pull request Jan 12, 2016
 - Custom style overrides
 - single and multi examples and tests
 - Downgraded sass naming convention errors to warnings until we can do inline/file based rules/exclusions
 - Added spacing between example components and removed unnecessary `<div>`s
 - `allowCreate` dependant on JedWatson/react-select#660
 - Removed unnecessary box shadow length properties
@dcwither
Copy link

👍

@throoze
Copy link

throoze commented Jan 19, 2016

Looking forward to this being implemented in the new version. Thanks for your efforts guys!

@mehcode
Copy link

mehcode commented Jan 22, 2016

Definitely excited about this 💯

@Craga89
Copy link

Craga89 commented Feb 27, 2016

Is this on the roadmap to be merged soon? Looking forward to using it in production! 👍

@wizhippo
Copy link

+1

1 similar comment
@gfeld
Copy link

gfeld commented Feb 28, 2016

+1

@lefnire
Copy link

lefnire commented Mar 23, 2016

Thanks @mitani for the workaround! For anyone looking for more details on said, here's a sample commit. I like this solution as it grants more flexibility, and works for multi={true|false} where allowCreate only works for multi={true}. You can see how it looks at jobpigapp.com -> register -> post job (just play with the location & tags).

Incidentally this PR is getting pretty old. I tried to fork & rebase but was a bit out of my league. I have to ask: is the only reason this hasn't been merged cosmetic concerns (aligning comments, } else { issues, etc)? That'd be a shame :/ there are some people hurting for a merge...

If it is rebase + style, I'd be happy to take a pass this week. If it's more, let us know

@cema-sp
Copy link

cema-sp commented Mar 28, 2016

Thank you, @lefnire and @mitani !
For those who use ES6 without lodash:

  filterOptions = (options, filter, values) => {
    // Filter already selected values
    let filteredOptions = options.filter(option => {
      return !(values.includes(option));
    });

    // Filter by label
    if (filter !== undefined && filter != null && filter.length > 0) {
      filteredOptions = filteredOptions.filter(option => {
        return RegExp(filter, 'ig').test(option.label);
      });
    }

    // Append Addition option
    if (filteredOptions.length == 0) {
      filteredOptions.push({
        label:  <span><strong>Create</strong>: {filter}</span>,
        value:  filter,
        create: true,
      });
    }

    return filteredOptions;
  };

@JuanCrg90
Copy link

+1

@bIgBV
Copy link

bIgBV commented May 10, 2016

Hey, I also want to add a +1 to this PR.

If it is only stylistic changes stopping from this being merged, I can get started on fixing them. We are using an older version of the library (0.9 to be specific) because it has the allowCreate option enabled. This is stopping us from upgrading to react v15.

@shaunr0b
Copy link

+1

@sethmcleod
Copy link

sethmcleod commented May 16, 2016

Thank you @cema-sp for that ES6 solution! One thing I noticed is that you can add && filter.length > 0 to the last conditional statement to prevent the Create label from showing if the user has selected all options but hasn't actually typed anything.

Also if you're rolling with this solution and you want to hide the word Create from showing up once the new label has been added to the selected options, I was able to handle that with some custom css.

if (filteredOptions.length === 0 && filter.length > 0) {
  filteredOptions.push({
    label:  <span><strong>Create: </strong>{filter}</span>,
    value:  filter,
    create: true,
  });
}
.Select-value-label strong {
  display: none;
}

Successfully using this in place of allowCreate for v1-beta in a react v15 build (with redux-form) and it's working great. Thanks to everyone who contributed!

@kopaygorodsky
Copy link

+1

@mehcode
Copy link

mehcode commented May 18, 2016

https://github.com/mehcode/react-select

^ I have no recollection of what I did but I think I based it off of this PR and did something. It gives me warnings on npm about missing peer dependencies but it does work with React 15.x

@wbecker
Copy link

wbecker commented May 25, 2016

This looks like it has stalled since December - are there only those few little stylistic comments in the PR holding it back? I could quite happily do these if it meant that this would get merged ASAP @omgaz!

@@ -1047,7 +1047,7 @@ describe('Select', () => {
describe('with allowCreate=true', () => {

// TODO: allowCreate hasn't been implemented yet in 1.x
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comment

@omgaz
Copy link
Contributor

omgaz commented May 26, 2016

THere's also a big missing chunk in an else if (this.props.allowCreate) {}, I'm not sure what vickyonit intended for this. It'd be awesome if someone would look into reviving this :) Thanks @wbecker

@gwyneplaine
Copy link
Collaborator

Thanks @vickyonit, this has since been superseded by #1187 which abstracts option creation to a Creatable HoC.

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.