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.

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

Quadrat: hide query pagination if there's no pagination #4272

Closed
wants to merge 5 commits into from

Conversation

jffng
Copy link
Contributor

@jffng jffng commented Jul 22, 2021

Changes proposed in this Pull Request:

This change relies on WordPress/gutenberg#33635 WordPress/gutenberg#35092

To test, activate this PR on a completely empty site or one with less than five posts ( so no pagination appears ).

Related issue(s):

Solves #4268

@jffng jffng requested review from kjellr and a team July 22, 2021 21:01
@jffng jffng linked an issue Jul 22, 2021 that may be closed by this pull request
@jffng jffng force-pushed the try/remove-extra-pagination-div branch from af0fb8d to ce96000 Compare September 29, 2021 15:38
@jffng
Copy link
Contributor Author

jffng commented Sep 29, 2021

I believe this is ready for a review.

To test, check out this PR and make sure pagination is appearing correctly. Then, adjust your site so that fewer than five posts exists. No pagination container or dividing line should be rendered.

@@ -1,6 +1,6 @@
<!-- wp:template-part {"slug":"header"} /-->

<!-- wp:query {"tagName":"main","layout":{"inherit":true},"queryId":1,"query":{"perPage":5,"offset":0,"postType":"post","categoryIds":[],"tagIds":[],"order":"desc","orderBy":"date","author":"","search":"","sticky":"","inherit":true}} -->
<!-- wp:query {"tagName":"main","layout":{"inherit":true},"queryId":1,"query":{"perPage":5,"offset":0,"postType":"post","categoryIds":[],"tagIds":[],"order":"desc","orderBy":"date","author":"","search":"","sticky":"","inherit":false}} -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to adjust inherit to false for pagination to work at all... This matches skatepark's index query too: https://github.com/Automattic/themes/blob/trunk/skatepark/block-templates/index.html#L6

@MaggieCabrera
Copy link
Contributor

Should we do this on search.html and query-diamond.php too?

@MaggieCabrera
Copy link
Contributor

This looks fine to me locally but wpcom still doesn't have the GB change

@MaggieCabrera
Copy link
Contributor

Screenshot 2021-10-01 at 10 04 46

I see this on dotcom, don't know if dotcom has the fix or not

@jffng
Copy link
Contributor Author

jffng commented Oct 1, 2021

Should we do this on search.html and query-diamond.php too?

Oh yeah, I will update there too.

I see this on dotcom, don't know if dotcom has the fix or not

Ah I see, shall we hold off on merging until the fix is there?

@MaggieCabrera
Copy link
Contributor

Yeah, we talked about holding this one until It lands since the bug is very minor

@scruffian
Copy link
Member

We should do this for all themes: #4805

@scruffian scruffian closed this Oct 7, 2021
@scruffian scruffian deleted the try/remove-extra-pagination-div branch October 7, 2021 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants