wp-calypso
wp-calypso copied to clipboard
Site Logs: Fix Pagination Page Number Tabs
Related to #92668
Proposed Changes
- Allows updating the currentPageIndex to the nextPageIndex instead of only incrementing or decrementing by one.
Why are these changes being made?
- On the Site Logs page, clicking on a pagination tab will presently only increment or decrement the current page by one. However, a user could reasonably expect that clicking on a pagination tab will take them to that page.
Testing Instructions
- Go to the Site Logs screen:
https://wordpress.com/site-logs/{site-url}/php - Ensure that there are at least 3 pages of logs. Additional logs can be added to the screen by changing the date range.
- Verify that pagination arrows will increment or decrement the current page by 1 (unless the current page is the first or last page).
- Verify that clicking on a pagination tab other than the current page tab will update the current page to the selected page.
Pre-merge Checklist
- [ ] Has the general commit checklist been followed? (PCYsg-hS-p2)
- [ ] Have you written new tests for your changes?
- [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
- [ ] Have you checked for TypeScript, React or other console errors?
- [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
- [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- [ ] For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?
Jetpack Cloud live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-117730&env=jetpack |
Automattic for Agencies live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-117730&env=a8c-for-agencies |
I haven't completed the pre-merge checklist because I don't have a testing environment set up. Requesting review from @Automattic/lego
This PR does not affect the size of JS and CSS bundles shipped to the user's browser.
Generated by performance advisor bot at iscalypsofastyet.com.
This PR modifies the release build for the following Calypso Apps:
For info about this notification, see here: PCYsg-OT6-p2
- blaze-dashboard
- command-palette-wp-admin
- help-center
- notifications
- odyssey-stats
- whats-new
- wpcom-block-editor
To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/site-log-pagination-only-increments-and-decrements-by-one on your sandbox.
@Aurorum Are you viewing the site on WordPress.com, or did you pull the branch for this PR and build Calypso locally. For info about building Calypso locally, check out the Getting Started guide here: https://github.com/Automattic/wp-calypso?tab=readme-ov-file#getting-started
On my end, the branch does allow navigating directly to a different tab instead of only being to increment or decrement by one page at a time. If not all of the logs are being displayed, then that could be a separate issue.
I saw in your screenshot that you have 10,000 log entries for the date range that you have selected. That's a very round number, so I wonder if there is a limit to the number of log entries that can be shown in a given date range.
@masperber I'm using Calypso Live, which is why I'm able to skip to the end now. :) I assume 10k is the cap, but either way, I don't think the issue is here that all the logs aren't being displayed. Instead, it seems to be that an API request is not made for the paginated logs. You should be able to see this even through skipping a couple of hundred logs (which seems a relatively low number for a WP.com site).
For instance, Page 8 isn't actually taking me to Page 8 of the logs - it's only taking me back one.
@Aurorum After looking into this further, I've discovered that the way that data is being loaded is that the 2nd page that you click onto will load the 2nd page of data, and the 3rd page that you click onto will load the 3rd page of data, etc, regardless of what number is shown in the tab. This is likely the reason why you could only increment and decrement by one.
In that case, I wonder if this change is even more confusing than the pagination tabs not working? I think the ideal fix would be to ensure that the API can send paginated responses so folks can navigate through different tabs - otherwise pagination doesn’t add much!
@Aurorum I agree. I'll revert this PR to a draft and continue the discussion on the issue thread.
This PR has been marked as stale. This happened because:
- It has been inactive for the past 3 months.
- It hasn't been labeled `[Pri] BLOCKER`, `[Pri] High`, `[Status] Keep Open`, etc.
If this PR is still useful, please do a trunk merge or rebase and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation.
If the PR is not updated (or at least commented on) in another month, it will be automatically closed.
This PR has been automatically closed as it has not been updated in some time. If you want to resume work on the PR, feel free to restore the branch and reopen the PR.