Arrange for pager to adapt itself to numberOfResults on each new query #465
Conversation
olamothe
requested review from
Dominionized,
dms1lva,
francoislg and
fbeaudoincoveo
|
I added @fbeaudoincoveo so you can review the doc comments on the deprecated option. |
src/ui/Pager/Pager.ts
Outdated
| import { QueryEvents, INewQueryEventArgs, IBuildingQueryEventArgs, IQuerySuccessEventArgs, INoResultsEventArgs } from '../../events/QueryEvents'; | ||
| import { | ||
| QueryEvents, INewQueryEventArgs, IBuildingQueryEventArgs, IQuerySuccessEventArgs, INoResultsEventArgs, | ||
| IDoneBuildingQueryEventArgs |
There was a problem hiding this comment.
Weird alignment => Either 1-line or multiple lines
src/ui/Pager/Pager.ts
Outdated
| this.bind.onRootElement(QueryEvents.newQuery, (args: INewQueryEventArgs) => this.handleNewQuery(args)); | ||
| this.bind.onRootElement(QueryEvents.buildingQuery, (args: IBuildingQueryEventArgs) => this.handleBuildingQuery(args)); | ||
| this.bind.onRootElement(QueryEvents.doneBuildingQuery, (args: IBuildingQueryEventArgs)=> this.handleDoneBuildingQuery(args)); |
There was a problem hiding this comment.
formatting: () => (space)
There are others in this file and PagerTest.ts
| lastValidPage = possibleValidPage; | ||
| } | ||
| } else if (this.currentPage > this.getMaxNumberOfPagesForCurrentResultsPerPage()) { | ||
| // Second scenario : Someone tried to access a non-valid page by the URL for example, which is higher than the current possible with the number of |
There was a problem hiding this comment.
An ogre ran away with the end of this comment. Get it back! 👹 ⚔️ 🏃
src/ui/Pager/Pager.ts
Outdated
| // before triggering/queuing the next query; | ||
| // if we cannot find a lastValidPage to go to, do nothing : this means it's a query that simply return nothing. | ||
| if (lastValidPage != null) { | ||
| Defer.defer(()=> this.setPage(lastValidPage)); |
There was a problem hiding this comment.
I'm not sure about deferring to execute after other callbacks. Can't you handle this directly in the querySuccess event?
There was a problem hiding this comment.
Indeed. There's actually a mechanism to retry the query, just like the DidYouMean feature use.
It's a flag on the noResults data that I can use to retry the query immediately.
src/ui/Pager/Pager.ts
Outdated
| */ | ||
| maxNumberOfPages: ComponentOptions.buildNumberOption({ defaultValue: undefined }) | ||
| maximumNumberOfResultsFromIndex: ComponentOptions.buildNumberOption({ |
There was a problem hiding this comment.
Specifies the maximum number of results that the index can return for any query.
Default value is 1000 in a Coveo index.
If this value was modified in your Coveo index, you must specify the new value in this option for the Pager component to work properly.
src/ui/Pager/Pager.ts
Outdated
| * | ||
| * @deprecated | ||
| */ | ||
| maxNumberOfPages: ComponentOptions.buildNumberOption({ |
There was a problem hiding this comment.
I'm not quite sure we should update the actual documentation of deprecated options.
We could rather add the new info next to the @deprecated tag.
So, we would have something like this:
Specifies the maximum number of pages to display if enough results are available.
This property is typically set when the default number of accessible results from the index has been changed from its default value of `1000` (10 results per page X 100 `maxNumberOfPages`).
Default value is `100`
@deprecated This is a deprecated option. The `Pager` now automatically adapts itself on each new query, so you no longer need to specify a value for this option. However, if the default maximum number of accessible results value was changed on your Coveo index, you should use the [`maximumNumberOfResultsFromIndex`]{@link Pager.options.maximumNumberOfResultsFromIndex} option to specify the new value.
On the generated doc site, the "deprecated" tag, along with its text, will be displayed in bright red at the very beginning of the option documentation. Should be clear enough :)
This allows the ResultsPerPage component to modify the number of results in a page on each new query, and the pager will correctly recalculate it's boundary.
This also ensure you cannot "go over" the max number of page and get stuck.