Remove Is Reskinned Flag: Remove from user and notice sections - Attempt 2 #99354

ddc22 · 2025-02-05T15:54:11Z

Proposed Changes

In addition to the PR above Switches to a theme based approach to style notices
Make the default styling of the component dark

Manually adds the light styling to any components that had the isReskinned flag
Adds the light styling for global notices
Converts the Notice component to typescript

#Testing Instructions

Go to: /subscribers/ try to add subscribers and make sure the notices work
Add an auto renew paid plan with credits and remove any payment methods and on the purchases page (/purchases/subscriptions/<site-id>) -> and plan purchase page, make sure the dark themed notice appears.
Check notices in other locations where applicable
In an incognito window reach the user step and make sure everything works well

github-actions · 2025-02-05T15:54:24Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132486

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132486&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132486&env=a8c-for-agencies

matticbot · 2025-02-05T16:02:55Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
notifications
odyssey-stats
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/remove-is-reskinned-from-notice-user-sections on your sandbox.

matticbot · 2025-02-05T16:04:37Z

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

App Entrypoints (~17 bytes removed 📉 [gzipped])

name                 parsed_size           gzip_size
entry-stepper              -84 B  (-0.0%)      -27 B  (-0.0%)
entry-subscriptions        +27 B  (+0.0%)       +7 B  (+0.0%)
entry-login                +27 B  (+0.0%)       +1 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~231 bytes removed 📉 [gzipped])

name                             parsed_size           gzip_size
jetpack-connect                       -408 B  (-0.0%)     -152 B  (-0.0%)
accept-invite                         -408 B  (-0.2%)     -147 B  (-0.3%)
signup                                 -71 B  (-0.0%)       -9 B  (-0.0%)
plugins                                +31 B  (+0.0%)       +3 B  (+0.0%)
media                                  +27 B  (+0.0%)       +6 B  (+0.0%)
themes                                 +24 B  (+0.0%)       +7 B  (+0.0%)
theme                                  +24 B  (+0.0%)       +6 B  (+0.0%)
staging-site                           +24 B  (+0.0%)       +4 B  (+0.0%)
site-tools                             +24 B  (+0.0%)       +4 B  (+0.0%)
site-settings                          +24 B  (+0.0%)       +6 B  (+0.0%)
settings-performance                   +24 B  (+0.0%)       +4 B  (+0.0%)
scan                                   +24 B  (+0.0%)       +6 B  (+0.0%)
marketplace                            +24 B  (+0.0%)       -2 B  (-0.0%)
jetpack-cloud-plugin-management        +24 B  (+0.0%)       +0 B
hosting-features                       +24 B  (+0.0%)       +5 B  (+0.0%)
hosting                                +24 B  (+0.0%)       +1 B  (+0.0%)
backup                                 +24 B  (+0.0%)       +5 B  (+0.0%)
a8c-for-agencies-sites                 +24 B  (+0.0%)       +6 B  (+0.0%)
a8c-for-agencies-plugins               +24 B  (+0.0%)       +0 B
site-purchases                         +18 B  (+0.0%)       -1 B  (-0.0%)
purchases                              +18 B  (+0.0%)       -3 B  (-0.0%)
write-flow                             +13 B  (+0.0%)       -5 B  (-0.0%)
stepper-user-step                      +13 B  (+0.0%)       +0 B
stats                                  +13 B  (+0.0%)       -2 B  (-0.0%)
site-overview                          +13 B  (+0.0%)       -6 B  (-0.0%)
site-marketing                         +13 B  (+0.0%)       -6 B  (-0.0%)
site-blocks                            +13 B  (+0.0%)       -1 B  (-0.0%)
settings-writing                       +13 B  (+0.0%)       -3 B  (-0.0%)
settings-security                      +13 B  (+0.0%)       -2 B  (-0.0%)
settings-reading                       +13 B  (+0.0%)       -7 B  (-0.0%)
settings-podcast                       +13 B  (+0.0%)       -2 B  (-0.0%)
settings-jetpack                       +13 B  (+0.0%)       -1 B  (-0.0%)
settings-discussion                    +13 B  (+0.0%)       -2 B  (-0.0%)
settings                               +13 B  (+0.0%)       -3 B  (-0.0%)
security                               +13 B  (+0.0%)       +0 B
purchase-product                       +13 B  (+0.0%)       -4 B  (-0.0%)
privacy                                +13 B  (+0.0%)       -1 B  (-0.0%)
posts-custom                           +13 B  (+0.0%)       -3 B  (-0.0%)
posts                                  +13 B  (+0.0%)       -4 B  (-0.0%)
people                                 +13 B  (+0.0%)       -4 B  (-0.0%)
pages                                  +13 B  (+0.0%)       +1 B  (+0.0%)
notification-settings                  +13 B  (+0.0%)       -1 B  (-0.0%)
newsletter-post-setup-flow             +13 B  (+0.0%)       -3 B  (-0.0%)
migrate                                +13 B  (+0.0%)       -7 B  (-0.0%)
me                                     +13 B  (+0.0%)       -1 B  (-0.0%)
marketing                              +13 B  (+0.0%)       -5 B  (-0.0%)
link-in-bio-post-setup-flow            +13 B  (+0.0%)       -3 B  (-0.0%)
jetpack-social                         +13 B  (+0.0%)       -1 B  (-0.0%)
jetpack-search                         +13 B  (+0.0%)       -1 B  (-0.0%)
jetpack-cloud-partner-portal           +13 B  (+0.0%)       -6 B  (-0.0%)
jetpack-cloud-agency-dashboard         +13 B  (+0.0%)       -5 B  (-0.0%)
import                                 +13 B  (+0.0%)       +0 B
home                                   +13 B  (+0.0%)       -3 B  (-0.0%)
help                                   +13 B  (+0.0%)       -1 B  (-0.0%)
google-my-business                     +13 B  (+0.0%)       +1 B  (+0.0%)
github-deployments                     +13 B  (+0.0%)       -3 B  (-0.0%)
export                                 +13 B  (+0.0%)       -2 B  (-0.0%)
email                                  +13 B  (+0.0%)       -2 B  (-0.0%)
earn                                   +13 B  (+0.0%)       -8 B  (-0.0%)
domains                                +13 B  (+0.0%)       -2 B  (-0.0%)
domain-connect-authorize               +13 B  (+0.1%)       -4 B  (-0.1%)
developer                              +13 B  (+0.0%)       -1 B  (-0.0%)
copy-site-flow                         +13 B  (+0.0%)       +0 B
concierge                              +13 B  (+0.0%)       -1 B  (-0.0%)
comments                               +13 B  (+0.0%)       -2 B  (-0.0%)
checkout                               +13 B  (+0.0%)       -4 B  (-0.0%)
build-flow                             +13 B  (+0.0%)       -3 B  (-0.0%)
add-ons                                +13 B  (+0.0%)       -6 B  (-0.0%)
activity                               +13 B  (+0.0%)       -7 B  (-0.0%)
account-close                          +13 B  (+0.0%)       -1 B  (-0.0%)
account                                +13 B  (+0.0%)       -1 B  (-0.0%)
import-hosted-site-flow                +12 B  (+0.0%)       -3 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.

Async-loaded Components (~226 bytes removed 📉 [gzipped])

name                                                      parsed_size           gzip_size
async-load-signup-steps-user                                   -494 B  (-0.2%)     -182 B  (-0.2%)
async-load-signup-steps-design-picker                           -45 B  (-0.0%)      -28 B  (-0.1%)
async-load-design-playground                                    +27 B  (+0.0%)       -2 B  (-0.0%)
async-load-design-blocks                                        +27 B  (+0.0%)       +5 B  (+0.0%)
async-load-design                                               +27 B  (+0.0%)       -2 B  (-0.0%)
async-load-calypso-post-editor-editor-media-modal               +27 B  (+0.0%)       +2 B  (+0.0%)
async-load-calypso-components-global-notices                    +27 B  (+0.3%)       +6 B  (+0.2%)
async-load-calypso-post-editor-media-modal                      +14 B  (+0.0%)       +8 B  (+0.0%)
async-load-store-app-store-stats                                +13 B  (+0.0%)       +1 B  (+0.0%)
async-load-calypso-my-sites-current-site-notice                 +13 B  (+0.0%)       +0 B
async-load-calypso-my-sites-current-site-domain-warnings        +13 B  (+0.0%)       -6 B  (-0.1%)
async-load-calypso-my-sites-checkout-modal                      +13 B  (+0.0%)       -3 B  (-0.0%)
async-load-calypso-lib-account-settings-helper                  +13 B  (+0.0%)       -1 B  (-0.0%)
async-load-calypso-blocks-editor-checkout-modal                 +13 B  (+0.0%)       -3 B  (-0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

ddc22 · 2025-02-06T17:06:01Z

@oswian @jjchrisdiehl really appreciate a review on this PR :)

oswian

I completed a first pass over the code and have left a couple of small comments. I also tested a few different instances of the Notice component and didn't spot any obvious issues relating to the changes.

However as the Notice component is used so widely, I definitely recommend having familiar eyes review the changes before merging.

Also just for context -- I'm wondering why particular instances of Notice deviate from the default theme. It doesn't seem ideal in terms of design consistency 🤔

oswian · 2025-02-07T01:49:29Z

client/blocks/time-mismatch-warning/index.tsx

@@ -43,7 +43,7 @@ export const TimeMismatchWarning: FC< ExternalProps > = ( {
 	const dismissClick = () => dispatch( savePreference( dismissPreference, 1 ) );

 	return (
-		<Notice status={ status } onDismissClick={ dismissClick }>


Should this instance of the notice actually use the default/dark theme, as it wasn't using the isReskinned prop?

Nice catch, distinctly remember seeing the is-reskinned class in the snapshot when running tests here but looks like it was from my own changes.

oswian · 2025-02-07T01:51:50Z

client/jetpack-connect/jetpack-connect-notices.jsx

@@ -269,7 +269,9 @@ export class JetpackConnectNotices extends Component {
 		if ( noticeValues ) {
 			return (
 				<div className="jetpack-connect__notices-container">
-					<Notice { ...noticeValues }>{ this.renderNoticeAction() }</Notice>


Similar again here. It didn't have the isReskinned prop enabled, so is the theme="light" is correct?

ddc22 · 2025-02-07T13:09:34Z

Also just for context -- I'm wondering why particular instances of Notice deviate from the default theme. It doesn't seem ideal in terms of design consistency 🤔

Good call out @oswian

I raised this on p2! pdvytD-Yh-p2
But I doubt it will get prioritised amidst other concerns.

…ce sections

ddc22 · 2025-02-07T18:03:44Z

However as the Notice component is used so widely, I definitely recommend having familiar eyes review the changes before merging.

@oswian given that now the approach is a themed based approach I think this is relatively safe to ship.

ddc22 requested a review from jjchrisdiehl February 5, 2025 18:40

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 5, 2025

ddc22 requested a review from oswian February 5, 2025 18:40

ddc22 marked this pull request as ready for review February 5, 2025 18:54

ddc22 requested a review from a team as a code owner February 5, 2025 18:54

ddc22 force-pushed the fix/remove-is-reskinned-from-notice-user-sections branch from 6442e7b to ded6575 Compare February 6, 2025 03:30

ddc22 requested a review from a team as a code owner February 6, 2025 05:59

oswian reviewed Feb 7, 2025

View reviewed changes

ddc22 added 9 commits February 7, 2025 09:54

Remove Is Reskinned Flag: Remove is reskinned flag from user and noti…

4452e9a

…ce sections

Fix snapshots

eff7b36

Convert to a theming approach

032013b

Cleanup and global notice

56a15ae

Add light theme to notices with isReskinned

0188708

Test fixes

8d2d753

Fix type

d183190

Fix: types

be0c908

Update README.md

9b88a15

ddc22 force-pushed the fix/remove-is-reskinned-from-notice-user-sections branch from 52c4221 to 9b88a15 Compare February 7, 2025 14:54

Revert class change

01b7ed3

ddc22 force-pushed the fix/remove-is-reskinned-from-notice-user-sections branch from c0fe2bc to 01b7ed3 Compare February 7, 2025 17:06

ddc22 merged commit 0ee9acb into trunk Feb 7, 2025
12 of 13 checks passed

ddc22 deleted the fix/remove-is-reskinned-from-notice-user-sections branch February 7, 2025 18:04

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 Feb 7, 2025

ddc22 mentioned this pull request Feb 10, 2025

Signup: Deprecate the concept of reskinned flows. #95419

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Is Reskinned Flag: Remove from user and notice sections - Attempt 2 #99354

Remove Is Reskinned Flag: Remove from user and notice sections - Attempt 2 #99354

ddc22 commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited by matticbot

Loading

matticbot commented Feb 5, 2025 •

edited

Loading

matticbot commented Feb 5, 2025 •

edited

Loading

ddc22 commented Feb 6, 2025

oswian left a comment

oswian Feb 7, 2025

ddc22 Feb 7, 2025

oswian Feb 7, 2025

ddc22 commented Feb 7, 2025

ddc22 commented Feb 7, 2025

Remove Is Reskinned Flag: Remove from user and notice sections - Attempt 2 #99354

Remove Is Reskinned Flag: Remove from user and notice sections - Attempt 2 #99354

Conversation

ddc22 commented Feb 5, 2025 • edited Loading

Proposed Changes

github-actions bot commented Feb 5, 2025 • edited by matticbot Loading

matticbot commented Feb 5, 2025 • edited Loading

matticbot commented Feb 5, 2025 • edited Loading

ddc22 commented Feb 6, 2025

oswian left a comment

Choose a reason for hiding this comment

oswian Feb 7, 2025

Choose a reason for hiding this comment

ddc22 Feb 7, 2025

Choose a reason for hiding this comment

oswian Feb 7, 2025

Choose a reason for hiding this comment

ddc22 commented Feb 7, 2025

ddc22 commented Feb 7, 2025

ddc22 commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited by matticbot

Loading

matticbot commented Feb 5, 2025 •

edited

Loading

matticbot commented Feb 5, 2025 •

edited

Loading