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

[select] feat: createNewItemPosition prop #4401

Merged
merged 7 commits into from
Nov 11, 2020

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Nov 6, 2020

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • new createNewItemPosition?: "first" | "last" prop on QueryList (defaults to "last" - current behavior)
  • add test and switch in example

Reviewers should focus on:

👋 hey buddy, hope you're well! this was fun to put together, and to revisit the project. i may have another select PR or two coming your way soon...

  • this allows the user to hit enter immediately to create a new item instead of having to find it at the bottom or hope for the query to exclude all existing items.

Screenshot

Blueprint_–_Documentation

@giladgray
Copy link
Contributor Author

hmm, this build fail does not happen locally. are you familiar with this one?
Error: error in C function svg-icon: Expected just one argument on line 39 of src/components/breadcrumbs/_breadcrumbs.scss

@mrijke
Copy link

mrijke commented Nov 9, 2020

@giladgray had the same issue locally on node 14 - switching back to node 12 seems to fix the problem.

@giladgray
Copy link
Contributor Author

i'm on node 12 locally, maybe that's why it works on my box? doesn't make sense how i can only edit select typescript files and core sass build is failing.

@adidahiya do you have insight here?

@adidahiya
Copy link
Contributor

@giladgray looks like Node 14 got promoted to the "lts" docker image. This should fix the build: #4405

@giladgray
Copy link
Contributor Author

🙌 great catch! thanks @adidahiya for the quick perfect fix 🚀

@giladgray
Copy link
Contributor Author

@adidahiya i don't mean to be a bother, but do you have a timeline for reviewing / releasing this? jonesing to use this prop ASAP!

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

LGTM! should have this in a release early next week

@adidahiya adidahiya changed the title [select] add createNewItemPosition prop [select] feat: createNewItemPosition prop Nov 11, 2020
@adidahiya adidahiya merged commit 00e0169 into palantir:develop Nov 11, 2020
@giladgray giladgray deleted the gg/query-list-create-position branch November 11, 2020 22:35
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

3 participants