feat(core): rename private and public methods and properties #349
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9922243:
|
| return hits; | ||
| }, | ||
| templates: { | ||
| item({ item }) { | ||
| return ` | ||
| <div class="item-icon">${searchIcon}</div> | ||
| <div> | ||
| ${reverseHighlightItem({ | ||
| item, | ||
| ${reverseHighlightHit({ |
There was a problem hiding this comment.
what do we win by calling this a hit, while we talk about item in the other spots? Is it because it's called getAlgoliaHits? Could that maybe be called getAlgoliaItems or search to remove the word hits?
There was a problem hiding this comment.
An item is a generic object manipulated by Autocomplete. A hit is an item transformed by Algolia which contains _highlightResult, etc. A hit is a search term and it reflects the Algolia brand.
| getItemInputValue: ({ item }) => item.label, | ||
| getItemUrl: () => undefined, |
There was a problem hiding this comment.
this is a good one, was inconsistent for sure!
| setQuery, | ||
| setSuggestions, | ||
| setCollections, |
There was a problem hiding this comment.
collection is a list of items internally, while it's still called getSuggestions outside (or do I misread it?)
There was a problem hiding this comment.
Nope, collections remain internal, except if you use the setters. getSuggestions is renamed to getItems, and the result of getItems is the items objet in a collection.
type Collection = { source, items };| itemInputValue: ReturnType< | ||
| InternalAutocompleteSource<TItem>['getItemInputValue'] | ||
| >; | ||
| itemUrl: ReturnType<InternalAutocompleteSource<TItem>['getItemUrl']>; |
There was a problem hiding this comment.
why are these prefixed by item, when we're already in an item? could inputValue & url be sufficient?
There was a problem hiding this comment.
They're coming from getItemInputValue and getItemUrl so it's easier to map the naming to these.
| const sections = state.suggestions.map((suggestion) => { | ||
| const items = suggestion.items; | ||
| const source = suggestion.source as InternalAutocompleteSource<TItem>; | ||
| const sections = state.collections.map((collection) => { |
There was a problem hiding this comment.
I wonder if getSources should be renamed to getCollections too (or is that not the same?)
There was a problem hiding this comment.
They're not the same, as explained in #349 (comment).
| setStatus('idle'); | ||
| setSuggestions(suggestions as any); | ||
| setCollections(collections); |
| const accumulatedSuggestionsCount = state.suggestions | ||
| .map((suggestion) => suggestion.items.length) | ||
| .reduce<number[]>((acc, suggestionCount, index) => { | ||
| const accumulatedCollectionssCount = state.collections |
There was a problem hiding this comment.
accumulatedCollectionssCount -> two ss, intended or a typo?
There was a problem hiding this comment.
Good catch—perhaps that announces a new broken key on my keyboard 🤪
See e162b51.
The Autocomplete naming were temporary and getting confusing because multiple concepts were mixed up. We spent some time rethinking the naming used in Autocomplete with @Shipow and came up with alternatives that make more sense to us.
This PR introduces multiple breaking changes that we'll need to document in the changelog—but that's fine since we're still in alpha.
Summary
itemUrl.getInputValue→getItemInputValuefor claritygetSuggestions→getItemsgetSuggestionUrl→getItemUrlsetHighlightedIndexis renamedsetSelectedItemIdfor clarity. "highlight" is a word that we use for highlighting and snippetting. This renaming removes this confusion.defaultHighlightedIndex→defaultSelectedItemIdNext
shouldDropdownShowwill also get renamed