-
Notifications
You must be signed in to change notification settings - Fork 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
Homepage replacement on AT: Fix default behaviour #65846
Homepage replacement on AT: Fix default behaviour #65846
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~22 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
12b5546
to
fe7f435
Compare
I don't fully understand what's happening here, but I noticed if I switch to a theme on an atomic site, and that theme hasn't been used on that site before, it uses this block of code which skips over the addition in this PR. Is that an issue? If the theme has been installed before, I do see it using the new code and overriding keepCurrentHomepage. |
You are right. the value should be set before that conditional because all AT sites are JP sites. I will modify and retest, nice catch!. |
@mreishus It's kind of contrived but I will try to explain. On Simple when you click on 'Activate' you are presented with a modal that gives you two options, preserve or replace your homepage. This option is controlled by a strangely named POST arg called Adding this PR (1046-gh-Automattic/wpcomsh) to
The thing is that if the Once the homepage modal is enabled in Atomic this will not be needed anymore. |
fe7f435
to
bbcde16
Compare
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.
Ok, makes sense. LGTM as I see no changes in behavior on either simple or atomic when changing themes.
The current behaviour on Atomic is to always preserve the home page.
This PR sets the
dont_change_homepage
totrue
on thethemes/mine
POST request that is made when theActivate
button is pressed. If this change is not added now, when this PR (1046-gh-Automattic/wpcomsh) is merged the default action would change on Atomic to "Always replace homepage" and we don't want that.Testing