add post-classes in the loop #30497
add post-classes in the loop #30497
Conversation
aristath
added
[Feature] Themes
aristath
changed the title
|
Size Change: 0 B Total Size: 1.43 MB ℹ️ View Unchanged
|
|
Code wise this looks good.
I feel the same way too, but theme authors should chime in to this. I saw an example use case noted in the issue here: #29308 (comment). Would this styling be applied to the editor as well? |
With this POC PR no... |
|
Looks good from my end too. For the long term, I'd like to see the class applied in the editor too if possible. I'm in favor of refactoring the classes, but back-compat might be a blocker. That's a separate issue though and should be brought up on WP Trac, I'd imagine. |
|
I pushed a commit, cleaning up the post-classes a bit.
Do these make sense? |
I'm still concerned about these changes as it could enable themes to add styles that can be confusing to the user (not visible in the editor) and I can't see a way for the user to change them through UI - could Global Styles could be involved here somehow and should they ? 🤔 --cc @nosolosw Maybe an alternative could be to keep just a couple of specific classes that would enable theme authors to have some refined handling in styling but apply them in the editor as well. I'm not sure about the proper handling of custom taxonomies, but we could start small by adding:
Would we need more classes from |
That is a fair point... It would indeed be confusing.
Some would argue that they are all necessary, otherwise, they wouldn't have been added in post-classes in the first place. |
|
If we are considering including specific classes instead of removing specific classes, then the sticky class needs to be included. |
|
Sticky posts can have a separate query (there's args for that already) so not as crucial IMO... Since they are pulled-out of the normal flow of posts, they are on a separate query which can have different styles. |
|
I see that #29308 was closed and I also don't have a good overall context about this topic, so it's probably best to hear what other people think. The thought that I have is that we're building the style system to be semantic (set this link color, set this other background color for this template part, and so on). It seems to me that the purpose of these sort of classes in classic themes should be absorbed by declaring the specific block supports and/or using theme.json to provide styles. |
|
To be honest I would keep the And I prefer the |
|
I realize there are a lot of things to take into account for the classes, which ones should be there, which ones should not etc... So in this PR I removed the classes filtering, and now it just adds the classes to |
|
Should we merge this one and then work on adding the classes on the editor side in a separate PR @ntsekouras? |
|
I stand by my earlier recommendation to move any decision and discussion on removing classes outside of this ticket, preferably on WP Trac. The goal here should be to use |
|
I'm still concerned about not having a way to see, but most importantly edit these styles in site editor. It feels like we should explore this first with other mechanisms as @nosolosw mentioned:
I fear that if this lands in core before finding a way to integrate/handle it properly, it could be taken for granted and create more problems to maintain than the benefits of the use case it solves for now. I'd like some more thoughts on this though @mtias , @mcsf |
|
I'm fine getting the class support in with the caveat that it's not an API contract within the editor. (We should probably also include |
aristath
force-pushed
the
add/query-loop-post-class
branch
from
21275a3 to
6efbd7d
Compare
|
Rebased the PR. I don't know if this is still something we want to do, but if we want it it's ready and mergeable. |
|
I'd like to still include |
Done 👍 |
aristath
force-pushed
the
add/query-loop-post-class
branch
from
6efbd7d to
88190fd
Compare
Description
This PR adds post-classes in the loop.
See #29308 for details.
I'm still not convinced we should do this... It feels too noisy. Over the years a lot of classes were added to these, so a random post would look something like this:
Things like
post,type-post,status-publish,hentryand possibly others too, don't make a lot of sense in an FSE context where queries are built by users... 🤔How has this been tested?
Tested in an FSE theme and made sure that posts inside a loop get the post-classes they previously had in classic themes.
Checklist:
*.native.jsfiles for terms that need renaming or removal).cc @justintadlock