Skip to content
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 Atomic: Toggle modal based on flag #65976

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

andres-blanco
Copy link
Contributor

@andres-blanco andres-blanco commented Jul 26, 2022

This PR enables the homepage modal on Atomic behind the themes/atomic-homepage-replace flag.

Note about the previews

image

**The current behaviour of the preserve homepage option doesn't seem to be working** Please make a comment on this PR about this subject. I think that it's best to remove the previews on the modal on Atomic on a subsequent PR.

Temporary steps until wpcomsh PR is merged

This is the wpcomsh PR that's needed in order for the modal to work: 1046-gh-Automattic/wpcomsh. If it's merged ignore the following steps and go directly to Testing.

  1. Set up a WoA site. /atomic/wpcom-dev-blogs/ on mc.
  2. Checkout the wpcomsh PR on your dev environment. 1046-gh-Automattic/wpcomsh
  3. run WPCOMSH_DEVMODE=1 make clean build
  4. run rsync -avze ssh --delete build/wpcomsh [YOUR WOA SITE]@sftp.wp.com:htdocs/wp-content/mu-plugins

Testing

  1. Apply this diff.
  2. Go to an Atomic site (or the WoA site you've created if the wpcomsh PR is not merged).
  3. Navigate to /themes/[YOUR_SITE]?flags=themes/atomic-homepage-replace.
  4. Select a theme and click on Activate.
  5. Verify you see the modal.
  6. Verify both options work as expected.
  7. Navigate to /themes/[YOUR_SITE].
  8. Select a theme and click on Activate.
  9. Verify there's no change from current Atomic behaviour and the homepage content is preserved.

Related to #65060 and #56869

@andres-blanco andres-blanco requested a review from a team July 26, 2022 16:18
@andres-blanco andres-blanco self-assigned this Jul 26, 2022
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 26, 2022
@matticbot
Copy link
Contributor

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~29 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
themes         +132 B  (+0.0%)      +29 B  (+0.0%)
theme          +132 B  (+0.0%)      +29 B  (+0.0%)
checkout       +132 B  (+0.0%)      +29 B  (+0.0%)

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@mreishus
Copy link
Contributor

Truth table of condition
2022-07-26_13-00

Copy link
Contributor

@mreishus mreishus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes made

  • When the 'themes/atomic-homepage-replace' flag is turned on, the keepCurrentHomepage = isSiteAtomic( getState(), siteId ) ? true : keepCurrentHomepage; override is turned off
  • When the 'themes/atomic-homepage-replace' flag is turned on, the modal also appears for WoA sites

Flag Off

  • The flag is currently off in all environments, so this is safe to deploy with no changes to production. However, we'd want to make sure everything is working properly before turning the flag on.

Flag On + wpcomsh#1046

In my testing, this is working properly with one minor issue. The modal correctly appears and I'm able to switch themes on my atomic site and both the replace and keep options work. However, I'm seeing the preview iframe not work properly, it renders a login screen instead of a preview:

2022-07-26_13-16

Possibly a firefox issue?

Summary

Good to deploy now, but I found one minor issue that is only revealed when the flag is turned on.

@andres-blanco andres-blanco merged commit 62b2b63 into trunk Jul 26, 2022
@andres-blanco andres-blanco deleted the add/homepage-modal-atomic branch July 26, 2022 19:01
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 26, 2022
@miksansegundo miksansegundo mentioned this pull request Feb 24, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants