-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Reader] Update limit of fetched tag posts from 20 to 7 in Reader #19882
[Reader] Update limit of fetched tag posts from 20 to 7 in Reader #19882
Conversation
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
Generated by 🚫 dangerJS |
We're correctly retrieving 7 items from the backend. 👍 A few things i noticed testing this PR: Top Filter UI Element Reverts to Discovery
** The animation expanding the Top UI is a bit laggy**
Side note: is the Top UI element still in progress? If i select Subscriptions, then select "XX Tags", it does says "Selected Site". |
Thanks for the review, @daniloercoli
I couldn't reproduce this one. I've tried in this branch and in the feature branch ( Screen.Recording.2024-01-12.at.10.35.49.AM.movIf it's reproducible and not caused by this change in the number of fetched posts (which at first sight doesn't seem related) I believe we should create a separate issue for it.
I also couldn't reproduce this one. Does it happen regularly or just the first time Screen.Recording.2024-01-12.at.10.38.51.AM.mov
Hmm, this part should be done already. Where did you see the Screen.Recording.2024-01-12.at.10.43.48.AM.mov |
@daniloercoli I believe the branch of this PR is a bit outdated and many changes came in the
Since that Filter top bar UI component (the Blogs and Tags pills) was still in development when this branch was opened. I just did a |
04cec98
to
3adf96a
Compare
Generated by 🚫 Danger |
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.
Fixes #19472
This was previously implemented and then reverted because the item animations when paging happened were wrong.
It seems like #19881 fixed the animations issue, which made me implement this again since there's a meaningful performance impact for the user by decreasing the number of fetched posts.
To Test:
Another way to test is opening the tag posts list from blogging prompts card:
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
--
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.UI Changes Testing Checklist: