-
Notifications
You must be signed in to change notification settings - Fork 123
Change post pattern on home.html
, update template slugs
#706
Change post pattern on home.html
, update template slugs
#706
Conversation
Thanks For PR @luminuu |
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.
Thank you @luminuu for the PR. Regarding the primary bug raised, I think this looks great.
I also am of course in favor of generally renaming home.html
to front-page.html
as mentioned, but there is another concern about that, see e.g. in #404 (comment).
I added a suggestion on how we can fix that below, but it's a bit more complicated. I'm curious to hear your thoughts, but if that change would go too far, we should probably limit this PR to only fixing the home.html
template (without renaming it to front-page.html
) and then continue exploring a solution for the renaming separately.
patterns/page-home-business.php
Outdated
<h2 class="wp-block-heading alignwide has-x-large-font-size" style="margin-top:0;line-height:1"><?php esc_html_e( 'Watch, Read, Listen', 'twentytwentyfour' ); ?></h2> | ||
<!-- /wp:heading --> | ||
|
||
<!-- wp:pattern {"slug":"twentytwentyfour/posts-3-col"} /--> |
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.
If this is used as part of front-page.html
while the WordPress site is configured to display a "static front page", it would look odd, since in that case there would only be the one page, with its excerpt shown in one of the three columns.
I'm not sure how we could fix this without modifying the WordPress template hierarchy. Honestly, I find this an odd configuration of WordPress core to choose front-page
regardless of whether that page is configured to show the blog or a static page.
Potentially we can use the frontpage_template_hierarchy
filter to customize this for TT4 to prefer a custom template, e.g. something like:
add_filter(
'frontpage_template_hierarchy',
function ( $templates ) {
if ( is_home() ) {
array_unshift( $templates, 'front-page-home.php' );
}
return $templates;
}
);
If we did that in functions.php
, we could have both a front-page.html
and front-page-home.html
template for the individual situations. front-page-home.html
could use the content as is here, while front-page.html
would instead replace those post columns with something similar to page.html
, i.e. displaying the content of a single individual page.
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.
I would like to see a video of how that would work with the Site Editor. Why is it a PHP file? How do users edit it?
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.
I'm getting more confused the more we talk about it. I assumed this was an issue so I renamed it, and now it seems to be a bigger issue that it's renamed? I'm fine reverting this change, but I have a hard time understanding what would be the best case, as it feels like whatever we do it's not. Adding code to fix something is not my preferred idea, because then you have to further explain how things work, and it will be harder for the average user to work with TT4. Since it's a legacy product, that's not what I want to achieve.
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.
I think this shouldn't be fixed in the theme
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.
we should probably limit this PR to only fixing the home.html template (without renaming it to front-page.html) and then continue exploring a solution for the renaming separately.
I'm good with this as well; to keep the scope/discussion focused.
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.
I agree with you all that fixing it in the theme would be merely a workaround for it being fixed in core, so I'm totally supportive of not doing that. We just won't be able to use this template as front-page.html
until that part is fixed in core. I'm happy to open a Trac ticket for that part, so that at least in the future it could be reconsidered.
I'm getting more confused the more we talk about it. I assumed this was an issue so I renamed it, and now it seems to be a bigger issue that it's renamed?
It's an issue that the blog template only shows the blog posts buried far down on the page, but that's a separate and minor issue compared to what I flagged in #705 / https://core.trac.wordpress.org/ticket/59759.
Given the above limitations of core, I think we should for now stick to only fixing the home.html
template, without renaming it. In other words, that won't address #404, but it will address #705 / https://core.trac.wordpress.org/ticket/59759.
With a static page selected in the reading settings: Ideally this should only display the title (optionally) and content of the page and nothing else. There is no way right now to encourage the user to update the template if they changed the pages under Settings > Reading instead of the Site Editor. |
I do not believe this is a theme problem: |
I one hundred percent don't believe this is a theme issue. We've had the front-page vs blog template debate since the dawn of block themes, and we are still having it with no solution in sight. These are our options and our concerns: home.html
front-page.html
The concerns with the query loop on our current iteration of blog are valid because we are using the blog template. But the page is very clearly not a blog. And the 3 featured posts are NOT a query loop in the classic sense of the word. They are not meant to show more than 3 posts, they just feature the last 3 posts. We are using the query block because it's a tool in our arsenal, it's never meant to care about the reading settings because this is a static page and I think that is OK. Not every WordPress theme needs to feature a home page that is meant for blogging, and this is a fundamental pillar this theme is built on, we are seeing a business site (though we provide an alternative blogging template and another for portfolios that are perfect replacements for the default one and that don't suffer from this issue). The fact that choosing to build a theme that deviates from the blogging paradigm and that it keeps encountering this kind of issues and concerns is pretty telling and something we need to collectively figure out, regardless of the end decision we make for TT4. What would happen if we removed the query block pattern from the blog altogether? Is that not a valid template for a theme to use as home page? (assuming we would rename it to front-page, since it really isn't a blog) Would that not be a valid design decision for a default theme? I think that is a little concerning. I'm not sure about the changes being made to the pattern in this PR, I would like the designer's opinion on this, particularly at this point in the release cycle. /cc @richtabor @jasmussen @scruffian @mikachan @jffng @beafialho |
My proposed solution |
@luminuu I'm going to work on this, I closed my other PR, and I will add this new pattern to yours if that's ok? I'm full on this this morning |
I implemented a new query loop that inherits following Bea's design. I didn't make it a pattern because that introduces new translatable strings. We can edit this on a point release to include them as a pattern for reusing, but that is not a blocker. Does this sound like a good solution to everyone? |
That's looking really nice, thank you! I'm going to test this later today. Would love it @felixarntz is able to give feedback on the question I had at the requested changes. |
Summarizing the changes: Current (trunk)Out of the box WordPressNo changes, as homepage display set to "latest posts": Homepage display to "Static page" (using sample page)The "home" changes to use the post content of the selected page (in this example, the default "Sample Page"). The page that was the homepage is now the "Blog Home": Proposed:Out-of-the-box WordPressNo changes, as homepage display set to "latest posts": Homepage display to "Static page"Create a "Blog" page, and assign the existing "Sample" page as the home page in homepage display: The "Blog" page: |
There's an opportunity to reduce the complexity here in core, but I do lean towards the proposal here. The out-of-the-box WordPress settings work better, and are less changes when switching the reading settings. |
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.
This looks good to me, let's try and find a solution in core so we can bring more variety to default themes front pages
I just have reverted the name change from |
I agree. FWIW I hadn't heard about this debate before, and only recently noticed that concern myself. But I think we're mixing up two different topics / issues here. The renaming of
I understand it's meant that way, but as long as it's in the
This is the only statement in your comment that I fundamentally disagree with. I get that not every website is for blogging, and not every theme needs to focus on blogging. But every theme, especially the default theme, needs to support the capabilities and features that WordPress offers by default. I agree there are several limitations in WordPress core that will need to be addressed to improve the situation for the future. But we cannot just break expected WordPress behavior. As long as WordPress has the setting that the home page shows the latest posts (with that being the default), we'll need to support that in a theme, especially the WordPress default theme. And again, this is a core problem more than a theme problem, but we shouldn't ignore that in the theme. Overall, also looking at your other PR, I think other than the one statement we're on the same page regarding that there are broader issues with how these expectations work in WordPress as a whole. For me, regardless of what we expect a home page to show, there is one technical limitation of WordPress core that prevents us from achieving what we want to achieve with TT4:
Obviously, we won't be able to address this core limitation for 6.4, so we'll have to find another way to work within these constraints and get to some trade-off solution. I am not sure I understand your current idea correctly, so it would be great if you could clarify that one more time. What do you propose? |
I closed that PR in favor of this one after talking to Bea and her proposing the new pattern that's included in this PR. I think front-page should be able to provide a static page regardless of query block being present at all but like you said, this is not something achievable right now. And I think that is something we should absolutely fix upstream, so this doesn't happen for future default themes and they can decide if the front page is or isn't focused on blogging. I like that this solution is contained and it doesn't alter the main concept for the theme. Thank you all for keeping an open discussion. |
Doesn't this proposal resolve both of these? Where you see the latest posts on the home page, while also having a home page that is not solely a blog? |
Based on the flows detailed in #706 (comment), the proposed PR (with the name change) seems to work better. Would appreciate feedback on the before/after, and other considerations. TLDR, if we use front-page.html, the home page stays the home page, when you change reading settings. You can still do whatever you'd like template editing wise, but you don't loose the homepage as you know it (as the setting reverts to using post content). |
…ry-and-breaks-navigation
…s-navigation' of github.com:WordPress/twentytwentyfour into 705-blog-template-homehtml-ignores-main-query-and-breaks-navigation
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.
In its current state, the PR addresses the problem outlined in #705, so this LGTM.
There's an outstanding conversation to address in #706 (comment), though I'll leave that to y'all to decide on. I left my feedback there for consideration.
I'm quickly summarizing the difference between the behaviour of If "Your homepage displays" is set to "Your latest posts"Both
|
I think with the changes to the query block, leaving this as home sounds good, then the user doesn't lose the settings from the sidebar to change the number of posts, they don't need to fish for those options. |
The PR in its current state does address this indeed. The remaining issues are:
That said, those two problems cannot reasonably be fixed by TT4 at this point, as they are limited by the current problematic WordPress core behavior. So I don't think we should get hung up on them for this PR. I've just opened a Trac ticket https://core.trac.wordpress.org/ticket/59767 for us to be able to at least address this limitation in the (hopefully near) future. |
@luminuu I agree this would be best left as is, in
Maybe worth adding: Unfortunately, WordPress will choose So I hope we can address this with more flexible template selection via https://core.trac.wordpress.org/ticket/59767 in the future. |
home.html
, update template slugs
Quick heads up I've renamed the title of this PR as the renaming has been reverted and will most likely not make it into the merge. |
I made the query block part into a pattern as discussed and I'm bringing this in. Thank you all for your patience getting this fixed |
Core PR here |
This update includes the following bugfixes: - Fix: Added a new block pattern for the home template that inherits the page query and fits the design of the home page. (more context on the theme repo: WordPress/twentytwentyfour#706) - Fix: Rely on parent theme data for block style. - Fix: Categories for some patterns. - Fix: Minor labeling issues Follow-up to [56999], [56951], [56813], [56764], [56716]. Props anlino, beafialho, desrosj, devmuhib, didierjm, fabiorubioglio, flixos90, hanneslsm, hellofromTonya, huzaifaalmesbah, ktaron, luminuu, mshowes, onemaggie, phillsav, poena, rajinsharwar, richtabor, shailu25. Fixes #59770, #59759. git-svn-id: https://develop.svn.wordpress.org/trunk@57036 602fd350-edb4-49c9-b593-d223f7449a82
This update includes the following bugfixes: - Fix: Added a new block pattern for the home template that inherits the page query and fits the design of the home page. (more context on the theme repo: WordPress/twentytwentyfour#706) - Fix: Rely on parent theme data for block style. - Fix: Categories for some patterns. - Fix: Minor labeling issues Follow-up to [56999], [56951], [56813], [56764], [56716]. Props anlino, beafialho, desrosj, devmuhib, didierjm, fabiorubioglio, flixos90, hanneslsm, hellofromTonya, huzaifaalmesbah, ktaron, luminuu, mshowes, onemaggie, phillsav, poena, rajinsharwar, richtabor, shailu25. Fixes #59770, #59759. Built from https://develop.svn.wordpress.org/trunk@57036 git-svn-id: http://core.svn.wordpress.org/trunk@56547 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This update includes the following bugfixes: - Fix: Added a new block pattern for the home template that inherits the page query and fits the design of the home page. (more context on the theme repo: WordPress/twentytwentyfour#706) - Fix: Rely on parent theme data for block style. - Fix: Categories for some patterns. - Fix: Minor labeling issues Follow-up to [56999], [56951], [56813], [56764], [56716]. Props anlino, beafialho, desrosj, devmuhib, didierjm, fabiorubioglio, flixos90, hanneslsm, hellofromTonya, huzaifaalmesbah, ktaron, luminuu, mshowes, onemaggie, phillsav, poena, rajinsharwar, richtabor, shailu25. Fixes #59770, #59759. Built from https://develop.svn.wordpress.org/trunk@57036 git-svn-id: https://core.svn.wordpress.org/trunk@56547 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This update includes the following bugfixes: - Fix: Added a new block pattern for the home template that inherits the page query and fits the design of the home page. (more context on the theme repo: WordPress/twentytwentyfour#706) - Fix: Rely on parent theme data for block style. - Fix: Categories for some patterns. - Fix: Minor labeling issues Follow-up to [56999], [56951], [56813], [56764], [56716]. Reviewed by flixos90, jorbin. Merges [57036] to the 6.4 branch. Props anlino, beafialho, desrosj, devmuhib, didierjm, fabiorubioglio, flixos90, hanneslsm, hellofromTonya, huzaifaalmesbah, ktaron, luminuu, mshowes, onemaggie, phillsav, poena, rajinsharwar, richtabor, shailu25. Fixes #59770, #59759. git-svn-id: https://develop.svn.wordpress.org/branches/6.4@57037 602fd350-edb4-49c9-b593-d223f7449a82
This update includes the following bugfixes: - Fix: Added a new block pattern for the home template that inherits the page query and fits the design of the home page. (more context on the theme repo: WordPress/twentytwentyfour#706) - Fix: Rely on parent theme data for block style. - Fix: Categories for some patterns. - Fix: Minor labeling issues Follow-up to [56999], [56951], [56813], [56764], [56716]. Reviewed by flixos90, jorbin. Merges [57036] to the 6.4 branch. Props anlino, beafialho, desrosj, devmuhib, didierjm, fabiorubioglio, flixos90, hanneslsm, hellofromTonya, huzaifaalmesbah, ktaron, luminuu, mshowes, onemaggie, phillsav, poena, rajinsharwar, richtabor, shailu25. Fixes #59770, #59759. Built from https://develop.svn.wordpress.org/branches/6.4@57037 git-svn-id: http://core.svn.wordpress.org/branches/6.4@56548 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Description
This PR fixes the issues around template names and query loops not inheriting the default loop, raised in #705 and #404. It replaces the original
posts-grid-2-col
pattern with theposts-3-col
pattern, wrapped in a group block to align with the rest of the layout and the original h2 headline. This fixes the issues raised in #705 with ignoring the main query and breaking the navigation.Renaming the
home.html
template file tofront-page.html
fixes the issue when a separate blog page is set, giving the users full control over the theme with the settings they make in Settings -> Reading. This was raised in #404.In addition, I've renamed the slugs of the template and page to the full file names, so they're the same as the other templates and patterns and are easier recognizable.
I'm aware this "breaks" aka reverts the original design, but I have to agree with @felixarntz that we should aim to not break the core functionality of the main query. I'm currently not aware what the capabilities of the query loop block in terms of design are and if there can be a different design for the query, but for now we should stick to this. If users want to go back to the original design with the three posts, they can totally do so and replace the main query, but then it's their active decision.
I'd be interested in getting feedback from @MaggieCabrera, @beafialho, @richtabor and anyone in @WordPress/block-themers.
Screenshots
The screenshot shows my test environment with the recent changes, including how 10 posts with images and an extra sticky post would look like on the main page, with the default WordPress settings.
Testing Instructions