Skip to content

Conversation

@arthur791004
Copy link
Contributor

@arthur791004 arthur791004 commented Apr 21, 2025

Related to WordPress/gutenberg#69081, #101265

Proposed Changes

  • The starter page modal was initially removed in WordPress/gutenberg#66836, but later restored by WordPress/gutenberg#69081. However, the registration of starter page patterns from Dotcom Patterns was removed in Automattic/jetpack#41479. As a result, the starter page modal may not appear if the active theme (e.g., TT1) doesn't include any matching patterns.
  • This PR proposes a fallback: if no starter patterns are available from the theme, the starter content will be inserted via the sidebar instead. Perhaps we can consider removing this test case, as it now aligns with Core’s behavior.

Why are these changes being made?

  • Fix e2e tests for GB 20.6.0

Testing Instructions

  • The Select page template test case should pass successfully

image

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 UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • 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-aUh-p2)?

@github-actions
Copy link

github-actions bot commented Apr 21, 2025

@arthur791004 arthur791004 self-assigned this Apr 21, 2025
@matticbot
Copy link
Contributor

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.

@arthur791004 arthur791004 requested a review from a team April 21, 2025 12:17
@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 Apr 21, 2025
@fushar
Copy link
Contributor

fushar commented Apr 21, 2025

Perhaps we can consider removing this test case, as it now aligns with Core’s behavior.

But how exactly? We need to click / dismiss the modal (e.g. we can't ignore the modal), otherwise we can't complete the Editor: Basic Page Flow test suite.

Copy link
Contributor

@fushar fushar left a comment

Choose a reason for hiding this comment

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

GUTENBERG_EDGE=true yarn workspace wp-e2e-tests test -- test/e2e/specs/editor/editor__page-basic-flow.ts

 PASS  specs/editor/editor__page-basic-flow.ts (27.182 s)
  Editor: Basic Page Flow
    ✓ Visit Pages page (1284 ms)
    ✓ Start a new page (5740 ms)
    ✓ Select page template (5285 ms)
    ✓ Template content loads into editor (86 ms)
    ✓ Open setting sidebar (70 ms)
    ✓ Set custom URL slug (566 ms)
    ✓ Close settings sidebar (44 ms)
    ✓ Publish page (4896 ms)
    ✓ Published URL contains the custom URL slug (1 ms)
    ✓ Published page contains template content (9 ms)

Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="desktop" ATOMIC_VARIATION="default" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="desktop" ATOMIC_VARIATION="private" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="desktop" ATOMIC_VARIATION="ecomm-plan" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="desktop" ATOMIC_VARIATION="wp-previous" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="mobile" ATOMIC_VARIATION="default" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="mobile" ATOMIC_VARIATION="private" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="mobile" ATOMIC_VARIATION="ecomm-plan" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="mobile" ATOMIC_VARIATION="wp-previous" TEST_ON_ATOMIC="true" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="desktop" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts
  • ✔️ yarn workspace wp-e2e-tests build && VIEWPORT_NAME="mobile" JETPACK_TARGET="wpcom-deployment" yarn workspace wp-e2e-tests test test/e2e/specs/editor/editor__page-basic-flow.ts

Seems to not break stuff for Jetpack tests.

@arthur791004 arthur791004 merged commit d589dde into trunk Apr 22, 2025
18 checks passed
@arthur791004 arthur791004 deleted the e2e/select-starter-page-if-need branch April 22, 2025 02:22
@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 Apr 22, 2025
@arthur791004
Copy link
Contributor Author

arthur791004 commented Apr 22, 2025

Thanks for helping to check jetpack tests 🙂

@anomiex
Copy link
Contributor

anomiex commented Apr 22, 2025

@arthur791004 It seems I screwed up my checks earlier. 🤦 Turns out this does break the tests with Jetpack on WoA.

  ● Editor: Basic Page Flow (Atomic: default) › Select page template

    locator.dispatchEvent: Timeout 10000ms exceeded.
    Call log:
      - waiting for locator('body.block-editor-page').locator('.editor-visual-editor').first().locator('iframe').contentFrame().locator('.editor-styles-wrapper').locator('.editor-post-title__input')

      56 |
      57 | 		// Every now and then, a block toolbar can cover the title, so we "force" using click events and the direct focus method.
    > 58 | 		await locator.dispatchEvent( 'click' );
         | 		              ^
      59 | 		await locator.focus();
      60 | 	}
      61 |

      at EditorGutenbergComponent.dispatchEvent (../../packages/calypso-e2e/src/lib/components/editor-gutenberg-component.ts:58:17)
      at EditorPage.addPatternFromSidebar (../../packages/calypso-e2e/src/lib/pages/editor-page.ts:428:3)
      at Object.<anonymous> (specs/editor/editor__page-basic-flow.ts:77:29)

@anomiex
Copy link
Contributor

anomiex commented Apr 22, 2025

Looks like all that's needed to fix it is to bring back the logic like

const inserterSelector = await editorParent.getByRole( 'listbox', { name: 'All' } );
const modalSelector = await editorParent.getByRole( 'listbox', { name: 'Block patterns' } );

await inserterSelector.or( modalSelector ).etc

@anomiex
Copy link
Contributor

anomiex commented Apr 22, 2025

Well, for desktop. Mobile still fails. Although we don't seem to run a mobile check for WoA.

@tbradsha tbradsha mentioned this pull request Apr 22, 2025
8 tasks
@anomiex
Copy link
Contributor

anomiex commented Apr 22, 2025

For the record: #102743

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.

5 participants