-
-
Notifications
You must be signed in to change notification settings - Fork 328
fix: search results limit #1996
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
base: main
Are you sure you want to change the base?
Changes from all commits
ec549f9
739a3a7
d6acbe4
4c453dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,7 +165,8 @@ export function useAlgoliaSearch() { | |
| return { | ||
| isStale: false, | ||
| objects: response.hits.map(hitToSearchResult), | ||
| total: response.nbHits ?? 0, | ||
| totalUnlimited: response.nbHits ?? 0, | ||
| total: Math.min(SEARCH_ENGINE_HITS_LIMIT.algolia, response.nbHits ?? 0), | ||
|
Comment on lines
+168
to
+169
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the Algolia cap contract consistent across helpers.
Suggested follow-up- const max = options.maxResults ?? 1000
+ const max = options.maxResults ?? SEARCH_ENGINE_HITS_LIMIT.algolia
…
return {
isStale: false,
objects: allHits.map(hitToSearchResult),
- total: serverTotal,
+ total: Math.min(max, serverTotal),
+ totalUnlimited: serverTotal,
time: new Date().toISOString(),
} |
||
| time: new Date().toISOString(), | ||
| } | ||
| } | ||
|
|
@@ -326,7 +327,8 @@ export function useAlgoliaSearch() { | |
| const searchResult: NpmSearchResponse = { | ||
| isStale: false, | ||
| objects: mainResponse.hits.map(hitToSearchResult), | ||
| total: mainResponse.nbHits ?? 0, | ||
| total: Math.min(SEARCH_ENGINE_HITS_LIMIT.algolia, mainResponse.nbHits ?? 0), | ||
| totalUnlimited: mainResponse.nbHits ?? 0, | ||
| time: new Date().toISOString(), | ||
| } | ||
|
|
||
|
|
||
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.
Expose the new disabled state visually on numbered page buttons.
The guard works, but the disabled page button still looks like a normal clickable button. That makes the new last-page restriction feel broken rather than intentional.
🎯 Proposed fix
Also applies to: 207-208