-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Render hidden span when on first/last page #40553
Conversation
Any idea if these blocks can be used outside of FSE or is that a requirement? Trying to figure out how to test this outside of the site editor for accessibility reasons. I wonder if a hidden span is really the best solution here. I'd like to test the front-end output... |
Yes it works in classic themes too, you can add the query to a page or post. Off topic: I wonder if each PR could have a generated link to Gutenberg.run with the option to use TT1 or TT2 for testing. |
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.
Looks good to me. Failing tests need fix though.
cc @ntsekouras and @mcsf since you gave input on #36681 |
This is simpler but isn't it the same "spacer GIF" like thing that the previous PR tried? |
In essence, yes. If we construct our layout logic assuming that all of the pieces will be there, and all of the pieces are actually there, then things work out as we expect (even if we can't see all of the pieces). That was the same idea @MaggieCabrera started on some time ago that the other PR grew from. The previous PR had lot of additional things done to account for other scenarios that made the change complicated. This is more straightforward and does nothing other than fixes the one problem that every layout I've every seen with this block has. |
@pbking Would it be possible to output the span, but not add the CSS inline but in the scss file, so that it can be overwritten? |
Yea that sounds like a good idea. I was just thinking of the opinionated design of having that element visible-but-faded (or otherwise modified). Styling via CSS would make that a fair bit easier. |
Thanks for the PR @pbking !
While this is true, we would be introducing too many inconsistencies here, with the simplest example if the Query Loop block inherits the global query. Having said that I still think that my previous comment applies here too.
We should definitely start exploring a solution like that. |
Any solution that doesn't allow rendering the elements because of (xxx) I believe is unhelpful. It's inconsistent that the element wouldn't be rendered because of (any reason). I disagree that this is a hack to accommodate but rather IS the behavior I would expect from this block. As-is there is no option to control the display behavior. With the change "invisible" is just the default style and fixes the biggest issue with the layout of this block, but as @carolinan mentioned changing this slightly easily introduces the ability for other options. If the default style needs to be Whatever the solution winds up being I don't think that "not rendering the element" should be a part of it. |
43be0ee
to
ec2b3f7
Compare
@carolinan for giggles I implemented this as you suggested. Now something like this could be done in a theme to display (and style) those elements:
|
Size Change: +75 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
I opened a draft PR with rendering a hidden element approach here, but needs more thinking.. I also started exploring a different layout to be used for the blocks as well and see where this leads us 😄
It's a complex thing how to handle this properly, but this is something that we need to explore. This might work well by removing some flexibility of the block, but most use cases might not need something more. |
I didn't find this mentioned in the existing feedback, but left-aligned pagination is negatively impacted in this branch, as the invisible Previous Page element shifts everything to the right — and vice-versa with right-aligned pagination shifted left by invisible Next Page elements: 40553-shifted-elements.movNik's alternative #40851 addresses this by inspecting the context's layout. |
I proposed a simpler fix here: #42764 |
What?
This change causes the query-pagination-next and query-pagination-previous blocks to render an invisible
<span>
when on the first/last page.This is an alternative to #36681 (which was merged and then removed) and
fixes #34997
NOTE: This is a much simpler solution than where #36681 landed and doesn't cover all the scenarios mentioned there (spacing around left/right aligned query pagination). However it fixes the most glaring issues and (I hope) fits well in the "what is the smallest that we can do that's helpful?". Especially since "space between" and "centered" are the two most likely design layouts to be used (and those don't look good today).
Why?
Currently the blocks render nothing when on the first/last page which causes issue with layout when on those pages.
How?
Add an additional
else{}
statement to the rendering logic to render the span with additionalvisibility:hidden
style attribute. The label is included in the span so that the size of the label will be taken into account during rendering (even though it is not visible).Screenshots or screencast
Before:
After:
@WordPress/block-themers