-
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 the site editor loading in multi-site installs #49861
Conversation
a46cf5e
to
28c371a
Compare
Size Change: +6 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
@youknowriad thanks for spoting this !
|
We also need to make sure this doesn't regress #48363. |
@Mamaduka yeah, as long as we use the url of the home page and not some "relative url" (like |
The testing I've done:
|
homepageId: | ||
siteData?.show_on_front === 'page' | ||
? siteData.page_on_front | ||
: null, | ||
url: siteData?.url, |
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.
Code-wise, this is in line with the other two places I've seen this in use: home-link and header-edit-mode.
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.
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.
@oandregal, the index endpoint data is defined here - https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/rest-api/class-wp-rest-server.php#L1255-L1266.
The change correctly uses home
instead of the url
.
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 works on my testing and code-wise seems fine.
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.
Thanks for this! Confirmed that it's working properly on the wpcom Multisite install now.
Will this fix be included in WP 6.2.1 ? |
Yes, that's the idea, it is labeled as such. |
Now that WordPress/gutenberg#49861 has landed on wpcom, we can remove our workaround of adding the query args ourselves.
Now that WordPress/gutenberg#49861 has landed on wpcom, we can remove our workaround of adding the query args ourselves.
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: a3358b2 |
WHAT
In multi-site installs, the site editor shows an infinite loader when accessing it. This PR should fix it.
🤖 Generated by Copilot at a46cf5e
WHY
The url property is not available in the /settings endpoint that we were using initially in multi site installs. This PR uses the index/base endpoint to retrieve the home page url instead.
HOW
🤖 Generated by Copilot at a46cf5e
Testing
I'd appreciate if folks have an easy way to setup a multi-site instance locally to check the fix there.