-
Notifications
You must be signed in to change notification settings - Fork 4.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
Fix static posts page setting resolved template #60608
Conversation
const siteSettings = getSite(); | ||
// First check if the current page is set as the posts page. | ||
const isPostsPage = +postId === siteSettings?.page_for_posts; | ||
if ( isPostsPage ) { |
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 is the main change. Rest of the changes are due to the added object deconstruction..
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.
The +postId is to make sure postId is an integer, in which cases it is not an integer could we not safely assume it is always an integer like we are assuming page_for_posts is an integer?
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 not really sure what is the flow that could result in a string and I left that untouched for now. ). Probably some cases for comparing post entities ids and the site settings - I think I had come across something like that in the past.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
const _homepageId = | ||
siteData?.show_on_front === 'page' && | ||
[ 'number', 'string' ].includes( typeof siteData.page_on_front ) && | ||
!! +siteData.page_on_front // We also need to check if it's not zero(`0`). |
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 is the fix for the infinite
loading in site editor.
Size Change: +528 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
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.
Things worked well on my tests and the issue is fixed 👍
const siteSettings = getSite(); | ||
// First check if the current page is set as the posts page. | ||
const isPostsPage = +postId === siteSettings?.page_for_posts; | ||
if ( isPostsPage ) { |
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.
The +postId is to make sure postId is an integer, in which cases it is not an integer could we not safely assume it is always an integer like we are assuming page_for_posts is an integer?
!! +siteData.page_on_front // We also need to check if it's not zero(`0`). | ||
? siteData.page_on_front.toString() | ||
: null; | ||
const _postsPageId = |
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.
Shouldn't we also check page_for_posts for zero?
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.
It doesn't matter for page_for_posts
because we don't use the value in a logical comparison(if
block).
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.
The changes test well for me.
I've also smoke-tested the post editor with a low-capability user and don't see any 404 errors caused by the getSite
selector call.
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
I just cherry-picked this PR to the update/fixes-for-wp-6-5-3 branch to get it included in the next release: f1abd8b |
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
What?
This PR aims to resolve: https://core.trac.wordpress.org/ticket/60836 (probably will need to create a GB issue..)
Additionally, this PR fixes a bug which causes 'infinite' loading in site editor when we have set a static
posts page
and no 'homepage`.Testing Instructions
posts page
and visit site editor. Observe that there is no 'infinite' loading.posts page
andhomepage
settings in reading settings.Tasks