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

Avoid useless search with minCharacter #10032

Closed
wants to merge 1 commit into from
Closed

Avoid useless search with minCharacter #10032

wants to merge 1 commit into from

Conversation

pierre-H
Copy link
Contributor

I noticed with the Admin Order Creation Plugin that event when there is no character, a search is done.

Q A
Branch? 1.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
License MIT

@pierre-H pierre-H requested a review from a team as a code owner December 14, 2018 09:25
@pamil pamil added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Potential Bug Potential bugs or bugfixes, that needs to be reproduced. labels Jan 2, 2019
I noticed with the Admin Order Creation Plugin that event when there is no character, a search is done.
@pamil pamil changed the base branch from master to 1.2 January 2, 2019 13:01
@pamil
Copy link
Contributor

pamil commented Jan 2, 2019

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "patch-2" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@Zales0123
Copy link
Member

Hello Pierre, could you take a look at the failing build and see what's causing it? Thanks! :)

@pierre-H
Copy link
Contributor Author

pierre-H commented Jan 4, 2019

@Zales0123 I don't understand why the build is failing … That's strange ...

@Zales0123
Copy link
Member

Do these scenarios work on your machine?

@pierre-H
Copy link
Contributor Author

pierre-H commented Jan 7, 2019

I didn't try on my machine : I used the GitHub online editor.

@pamil
Copy link
Contributor

pamil commented Jun 13, 2019

The build is failing because the test client does not fill in the field so there are no characters.
Anyway, I think we fixed it with #10404, limiting default results to 25 makes the search much more performant and also allows for selecting a choice without filling the input (which might be helpful eg. if you have not that much taxons in your app).

Thanks for the PR and sorry for such a delay.

@pamil pamil closed this Jun 13, 2019
@pierre-H pierre-H deleted the patch-2 branch June 13, 2019 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Potential Bug Potential bugs or bugfixes, that needs to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants