-
Notifications
You must be signed in to change notification settings - Fork 215
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
Replace "Over..." language with more precise "Top..." #4509
Conversation
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 like this approach; LGTM.
The API no longer returns an abstract, unreachable limit of 10k results. With this comment, we will show a precise number of actually reachable results, while still clarifying that these (may) be only the _top_ <x> results for that query (and not _all_ <x> results for that query).
3053277
to
bd04171
Compare
That storybook visual regression test might be flaky. Re-running it to confirm. The visual diff shows that the test failed due to the icon never loading. Here's the failed run for the record: https://github.com/WordPress/openverse/actions/runs/9575642719/job/26400882637?pr=4509 And the test results zip |
Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR: @obulat Excluding weekend1 days, this PR was ready for review 4 day(s) ago. PRs labelled with medium urgency are expected to be reviewed within 4 weekday(s)2. @sarayourfriend, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
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.
Everything works great, thank you for updating this so fast!
Fixes
Follow up to #4476 by @obulat
Description
The API no longer returns an abstract, unreachable limit of 10k results. With this comment, we will show a precise number of actually reachable results, while still clarifying that these (may) be only the top results for that query (and not all results for that query).
This is one option for resolving this, as described in the linked discussion.
Testing Instructions
Checkout the branch and run
./ov just p frontend dev
and visit http://localhost:8443. Search for something that will return heaps of results, like "birds". Now try something more niche like "big green bird" (which mostly just reduces audio results). Notice for each of these the text in the search bar and in the media type buttons says "Top results/".Do this for each of the three types of searches (all media, image, audio), and confirm you get the same "Top " text when the results are at the "max" value for any of the individual media types (240).
Checklist
Update index.md
).main
) or a parent feature branch.just catalog/generate-docs
for catalogPRs) or the media properties generator (
just catalog/generate-docs media-props
for the catalog or
just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin