Add a GS limit notice in the Global Styles sidebar

[BogdanUngureanu]

Just like the other Global Styles limit notices, the new notice is shown only for free and personal plans. The implementation is based on the existing noticed by refactoring the logic we used in the pre-save GS notice.

Related to https://github.com/Automattic/dotcom-forge/issues/1233

Proposed Changes

• Add a new notice in the Global Styles sidebar based on the ComplementaryArea/core/edit-site slot.

Testing Instructions

• Use ETK build from this branch on your Sandbox
• On a free Simple Site, go to site editor and click on the GS button
• Notice that the upgrade nudge is not displayed
• Make a GS change
• Go to Global Styles sidebar and notice that we now display an upgrade nudge on all pages.

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

[github-actions]

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[BogdanUngureanu]

There's a TS error because it fails to recognize __experimentalGetCurrentGlobalStylesId, getEditedEntityRecordand __experimentalGetDirtyEntityRecords.

TypeScript fails me :(. I'll make the adjustments after the code review.

[matticbot]

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit add/global-styles-notice on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

[dsas]

This change means that the sidebar will always be taller than the page - we're displaying content above the global styles sidebar which has a css rule of .edit-site-global-styles-sidebar { min-height:100% }.

In turn this means that due to an existing GB bug, a horizontal scrollbar will always be displayed for some combinations of browsers and scrollbar settings (see https://github.com/WordPress/ gutenberg/issues/29912#issuecomment-900000689).

Here's a video of it in action:
https://user-images.githubusercontent.com/93301/227530057-1d7b82a6-8dfb-4e95-923b-eb1e553ade9c.mp4

It'd be nice if we could so something about this but I don't think it's a blocker. The same thing happens when looking at the global styles block list.

[mmtr]

This work well!

It'd be great if the notice wouldn't have the top margin, so it's right at top the sidebar.

I also consider that the current pre-save notice is no longer relevant, so I'd probably get rid of it (and possibly re-use the same copy in the new permanent notice since it looks a bit clearer).

This change means that the sidebar will always be taller than the page

IMO, I think we should fix this before shipping it, and ensure that the sidebar doesn't have the scrollbar if it is not needed:

Screen.Recording.2023-03-30.at.16.03.05.mov

[BogdanUngureanu]

It'd be great if the notice wouldn't have the top margin, so it's right at top the sidebar.

Fixed it! :)

I also consider that the current pre-save notice is no longer relevant, so I'd probably get rid of it (and possibly re-use the same copy in the new permanent notice since it looks a bit clearer).

I think there's still value in showing the notice in the PrePublish panel too. We've seen that upgrade notices in the editor historically don't convert well; one theory we had was that the user might not want to interrupt their work on their site to upgrade to a paid plan.

IMO, I think we should fix this before shipping it, and ensure that the sidebar doesn't have the scrollbar if it is not needed:

I've tried again to remove the scrollbar but without much luck (since the changes would require at least overriding the existing styling from Gutenberg core - which will make the implementation less robust).

IMO I think this styling should be handled in Core because the slot we are using (which is a public API) doesn't accommodate multiple UI elements in that section.

[BogdanUngureanu]

Looks like the tests are failing :(

[BogdanUngureanu]

Looks like the merge is not blocked anymore, but I still have two issues:
The WPCOM_Global_Styles_Test::test_wpcom_block_global_styles_frontend is failing on line 30:

Failed asserting that null matches expected 'hotpink'.

/var/www/html/wp-content/plugins/editing-toolkit-plugin/wpcom-global-styles/test/class-wpcom-global-styles-test.php:30

There's a JS error in the console when you click on a GS menu item: Looks like it's unrelated. It's happening for Atomic sites on production.

caught TypeError: Cannot read properties of undefined (reading 'find')
    at Object.trackGlobalStylesMenuSelected [as handler] (wpcom-block-editor-global-styles-menu-selected.js:18:1)
    at delegate-event-tracking.js:116:1
    at Array.forEach (<anonymous>)
    at __webpack_modules__../src/wpcom/features/tracking/delegate-event-tracking.js.__webpack_exports__.default (delegate-event-tracking.js:111:1)
    at HTMLDocument.delegateCaptureListener (tracking.js:865:1)

[mmtr]

I found a regression, not sure if that is related with the test failure.

Previously, you could switch between style variations and the GS panel remained open:

Screen.Recording.2023-03-31.at.17.09.36.mov

However, with these changes, the GS panel is closed automatically after selecting a style variation:

Screen.Recording.2023-03-31.at.17.10.36.mov

EDIT: The same happens if you try to change any other style such as a color or a typography.

[BogdanUngureanu]

I found a regression, not sure if that is related with the test failure.

Yeah, it's a known issue with the approach (more like a limitation)... it was already discussed about it somewhere. The problem is that when the slots change, the slot-fill generic component does a forceUpdate on all fills, which means the local state is reset for all fills and the user is moved back to RootScreen.

This happens in two scenarios, as you've noticed:

• When the notice is shown the first time
• When the notice is removed (e.g. when you revert back to default state, non-gated, like you did)

Sadly I haven't found a solution for this (and I doubt we can do something about it) :(

[mmtr]

Nice, it works great now!

I noted that on mobile viewports the notice is partially duplicated but we can address that if needed on a follow-up (I still think we can/should remove the pre-save notice).