-
Notifications
You must be signed in to change notification settings - Fork 26
Remove query-{id}-page pagination parameter while searching #22
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?
Conversation
Search does not work on paginated pages, therefor remove it. This should be OK because search is expected to search from all items, not just from paged items. Fixes humanmade#21.
roborourke
left a comment
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.
Thanks for this, I think the approach needs a small refinement but otherwise it's great. I've left some comments inline.
|
|
||
| if ( $block->context['query']['inherit'] ) { | ||
| $query_var = sprintf( 'query-%s', $attributes['taxonomy'] ); | ||
| $page_var = 'page'; | ||
| $base_url = str_replace( '/page/' . get_query_var( 'paged' ), '', remove_query_arg( [ $query_var, $page_var ] ) ); | ||
| } else { | ||
| if ( empty( $block->context['query']['inherit'] ) ) { | ||
| $query_id = $block->context['queryId'] ?? 0; | ||
| $query_var = sprintf( 'query-%d-%s', $query_id, $attributes['taxonomy'] ); | ||
| $page_var = isset( $block->context['queryId'] ) ? 'query-' . $block->context['queryId'] . '-page' : 'query-page'; | ||
| $base_url = remove_query_arg( [ $query_var, $page_var ] ); | ||
| } else { | ||
| $query_var = sprintf( 'query-%s', $attributes['taxonomy'] ); | ||
| $page_var = 'page'; | ||
| $base_url = str_replace( '/page/' . get_query_var( 'paged' ), '', remove_query_arg( [ $query_var, $page_var ] ) ); | ||
| } |
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.
This is appears to just be flipping the logic around and not actually changing anything, was it solving something specific?
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.
@roborourke Yeah, not sure what that is :)
Looks like removing pagination while doing the search should happen in view.js.
| // Remove pagination when performing a search | ||
| // for your specific pagination format: query-{id}-page | ||
| const searchParams = url.searchParams; | ||
| [...searchParams.keys()].forEach(param => { | ||
| if (param.match(/query-\d+-page/) || param === 'paged') { | ||
| searchParams.delete(param); | ||
| } | ||
| }); |
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 think it should be possible to scope this to the current query loop block as this will reset pagination for all query loops on the page, and they should be able to work independently. E.g. if name === 's' remove the paged param, if name !== 's', remove name.replace( '-s', '-page' ).
Search does not work on paginated pages, therefor remove it.
This should be OK because search is expected to search from all items, not just from paged items.
Fixes #21.