Skip to content
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.

Sign up for GitHub

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

Jump to bottom

UI: Reset page query param on search #4822

Merged
merged 4 commits into from
Nov 1, 2018
Merged

UI: Reset page query param on search #4822

merged 4 commits into from
Nov 1, 2018

Conversation

DingoEatingFuzz
Copy link
Contributor

Fixes a search + pagination bug.

Current behavior

  1. Visit a page with a paginated table
  2. Click "next page"
  3. Search for anything
  4. See that there are supposed to be results, but none are shown because you are still on page 2

New behavior

  1. Visit a page with a paginated table
  2. Click "next page"
  3. Search for anything
  4. See the results because searching takes you back to page 1

@DingoEatingFuzz
Add onChange hook to the search-box component
badb6c4
@DingoEatingFuzz
Add a resetPagination method to the searchable mixin
f7ad2ce 
Searchable can be used without pagination, but reseting pagination
is more a function of search than pagination insofar as if you
add search to a page, you are also going to want automatic pagination
resetting.
@DingoEatingFuzz
Add the resetPagination hook to search boxes across the app
44e33d0
@DingoEatingFuzz
Test coverage for resetPagination
b00916a
@DingoEatingFuzz DingoEatingFuzz requested a review from a team October 31, 2018 19:46
alisdair
alisdair approved these changes Nov 1, 2018
View reviewed changes
Copy link
Member

@alisdair alisdair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@johncowen
Copy link
Contributor

Hey,

I was just trying this out with a smallCluster changed to 30 jobs. I was wondering if, as a extra bit of detail, it would only jump back to page 1 if there are no items on the page I'm currently on. So say I search and get 20 results, and click to go to page 2, then refine my search slightly by adding a further letter and I get 15 results, do I want to be auto navigated back to page 1, or would I rather stay on page 2? To put it shorter, should it only go back to page 1 if there are no items on the page you are on?

Refinement suggestion more than anything.

Thanks

@DingoEatingFuzz
Copy link
Contributor Author

IMO it should always go back to page one. Since the search results are sorted by relevancy, refining the search and not being one page one means you aren't seeing the most relevant results.

@DingoEatingFuzz
Copy link
Contributor Author

I just double checked what various sites do (google, duckduckgo, gmail, github) and they all reset pagination.

@DingoEatingFuzz DingoEatingFuzz merged commit 58230b3 into master Nov 1, 2018
@DingoEatingFuzz DingoEatingFuzz deleted the b-ui-reset-page-param branch November 1, 2018 20:20
@johncowen
Copy link
Contributor

Cool, good point, thanks for looking into it!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alisdair alisdair alisdair approved these changes

Assignees
No one assigned
Labels
theme/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DingoEatingFuzz @johncowen @alisdair