-
Notifications
You must be signed in to change notification settings - Fork 333
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
docs(createQuerySuggestionsPlugin): rewrite docs #468
Conversation
Broken unit tests are caused by 752afd8 which broke the build. |
There are additional params since the doc creation: autocomplete/packages/autocomplete-plugin-query-suggestions/src/createQuerySuggestionsPlugin.ts Lines 23 to 38 in 612b040
|
Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
59e319c
to
99186af
Compare
Addressed in 4d429df. |
|
||
> `number` | ||
|
||
The number of items to display categories for. |
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.
I wrote this description myself quickly but not sure it's very comprehensible. Wdyt?
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 you quickly describe it to make sure it's clear to me?
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.
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.
Got it! It's how many items get categories. I'm wondering if the name of the parameter is optimal though. Now that I know what it is, categoriesLimit
still doesn't convey that idea. Have you thought of something more descriptive like itemsWithCategories
? It doesn't convey as much that it expects a number
but it's clearer.
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.
Yeah I'm not convinced by the name either.
How about those?
numberOfItemsWithCategories
numberOfCategoriesPerItem
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.
I find numberOf
superfluous, the type conveys this information already.
itemsWithCategories
categoriesPerItem
This is simpler.
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.
OK—I'll take care of that in another PR.
Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
This rewrites the
createQuerySuggestionsPlugin
docs.