[E2E] Embed block fixes

[fullofcaffeine]

Changes proposed in this Pull Request

All embed blocks (the ones touched by our E2E tests at least) now follow a similar naming/classification standard. They now all have the same ariaLabel and embed prefix for the .editor-block-list-item CSS class.

This fixes the selectors so that the elements can be found on the page.

Outstanding questions

• Is it a good idea to have identical aria-labels for these blocks? They are all "embed" blocks, but they serve different purposes.

[matticbot]

Test live: https://calypso.live/?branch=fix/gb-8.9-coblocks-2.2.2-e2e

[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.

[fullofcaffeine]

I don't know if it'll be enough to just change the embed blocks to be selected by the same aria-label as I've done in the first commit here. It seems to be used to select them on the page for a scenario where multiple embed blocks are present.

I'm waiting for the tests to finish, but if they continue to fail, we will need to think of another way to select each block or re-organize the scenario so that a single embed block is tested per time, in the editor.

UPDATE: Even if the tests pass, we might still need to rethink these scenarios or ask ourselves if we really want to keep the same aria-label for them.

I've looked at the three blocks in the editor, and they now have pretty much the same markup. To test, open a test edge site and add the following blocks to the page: Instagram, Twitter, and Youtube. Now, inspect their DOM structure in the Elements inspector. You'll see all of them have pretty much the same markup, with the only difference being the ID hash. Screenshot follows:

[ockham]

I've looked at the three blocks in the editor, and they now have pretty much the same markup. To test, open a test edge site and add the following blocks to the page: Instagram, Twitter, and Youtube. Now, inspect their markup in the Elements inspector. You'll see all of them have pretty much the same markup, with the only difference being the ID hash.

Yeah, I remember seeing that when I was working on some oEmbeds related stuff in Gutenberg the other day. I agree that we'll probably want to differentiate more between them. This is something we'll need to address in Gutenberg though, and it likely won't make it into v8.9, so we also need a workaround for our e2e tests.

Edit: The embed block tests are actually passing, it's the tests for embedding a URL that are failing now:

• wp-calypso/test/e2e/specs/wp-calypso-gutenberg-post-editor-spec.js

  Line 1169 in b1f4e8f

  this.instagramEditorSelector = '.wp-block-embed-instagram';

• wp-calypso/test/e2e/specs/wp-calypso-gutenberg-post-editor-spec.js

  Line 1178 in b1f4e8f

  this.twitterEditorSelector = '.wp-block-embed-twitter';

• wp-calypso/test/e2e/specs/wp-calypso-gutenberg-post-editor-spec.js

  Line 1186 in b1f4e8f

  this.youtubeEditorSelector = '.wp-block-embed-youtube';

The class name for all of those is wp-block-embed now (which unfortunately isn't enough to differentiate between them when we insert all of them into the same post).

[ockham]

I think we can change those selectors to look for and <iframe />s with the title attribute set to

• Embedded content from instagram.com,
• Embedded content from twitter, and
• Embedded content from youtube.com, respectively.

(Those are coming from https://github.com/WordPress/gutenberg/blob/a610923b7728f1192f9745a55390a7abb0df576e/packages/block-library/src/embed/embed-preview.js#L78)

[fullofcaffeine]

I think we can change those selectors to look for and <iframe />s with the title attribute set to

* `Embedded content from instagram.com`,

* `Embedded content from twitter`, and

* `Embedded content from youtube.com`, respectively.

(Those are coming from https://github.com/WordPress/gutenberg/blob/a610923b7728f1192f9745a55390a7abb0df576e/packages/block-library/src/embed/embed-preview.js#L78)

I like that idea, but it seems this changeset is not part of 8.9? I couldn't find this attribute on embed blocks in a test site running 8.9. Am I missing anything?

That being said, I did start working towards this, there's some initial work I committed today. The upgrade test is still failing just when it starts the edge part of the test, possibly related to https://github.com/Automattic/wp-calypso/blob/master/test/e2e/lib/gutenberg/gutenberg-editor-component.js#L203, but I'll look tomorrow (or you can if you're feeling adventurous).

[ockham]

Oh, I was thinking we'd need to change the selectors only in the file I was mentioning, https://github.com/Automattic/wp-calypso/blob/38b51c6665cd2029a6fe6d0eca42893e7eefcf1c/test/e2e/specs/wp-calypso-gutenberg-post-editor-spec.js. Let me see if that does the trick.

[ockham]

Yep, that worked 🥳 The 'Click to Tweet' block seems to be the last remaining issue now.

[ockham]

Yep, that worked The 'Click to Tweet' block seems to be the last remaining issue now.

Found the relevant screencast in test artifacts: https://98647-102822243-gh.circle-artifacts.com/2/wp-calypso/test/e2e/screenshots/videos/136-2020-09-03T11-58-44.mpg

Upon attempting to leave the editor, there's a "Leave app? Changes you made may not be saved" alert (even right after saving the post). Screengrab from the screencast:

Seems like dirty state detection is broken 😕

cc/ @david-szabo97 since we've recently touched the UnsavedChangesWarning component a bit (specifically: WordPress/gutenberg#24719); although the warning coming from that component reads differently 🤔

[ntsekouras]

Yep, that worked 🥳 The 'Click to Tweet' block seems to be the last remaining issue now.

I don't have all the context here, but when working with the embed block variations, I used snapshots to make sure they were correct. (https://github.com/WordPress/gutenberg/blob/87c4dc536f52254975b4b5e192e36f7d98b6f0b7/packages/e2e-tests/specs/editor/various/embedding.test.js#L211)

Since @ockham found a workaround for some of the tests, if the click to edit is still a problem, another approach would be to select the tweet block with key navigation events. Would that apply here?

[ockham]

Yep, that worked 🥳 The 'Click to Tweet' block seems to be the last remaining issue now.

I don't have all the context here, but when working with the embed block variations, I used snapshots to make sure they were correct. (https://github.com/WordPress/gutenberg/blob/87c4dc536f52254975b4b5e192e36f7d98b6f0b7/packages/e2e-tests/specs/editor/various/embedding.test.js#L211)

Since @ockham found a workaround for some of the tests, if the click to edit is still a problem, another approach would be to select the tweet block with key navigation events. Would that apply here?

Thanks Nik. A number of misunderstandings here, I'll try to clarify:

• The problem with the embeds has been resolved (by changing e2e selectors).
• The remaining problem is with the "Click to Tweet" block (not the Twitter embed). That block is part of the CoBlocks collection of blocks that we're running on WP.com Furthermore, the failing test is checking for the presence of the rendered block on the frontend (rather than the editor). This seems to happen because we get stuck when attempting to leave the editor because dirty state detection seems broken (see my previous comment).

[ockham]

The reason for the "Changes you made may not be saved" warning after publishing a post that contains a "Click to Tweet" CoBlock seems to be that the block has a url attribute that contains to the current post's URL (plus that URL is also found in the block's content). If the block is inserted into a newly created post (as it is in e2e tests), that url attribute will be set to something like https:\/\/example.com\/2020\/09\/03\/auto-draft\/. If the post is saved, it will be updated to use the post's slug (which is based on the post title), e.g. https:\/\/example.com\/2020\/09\/03\/click-to-tweet-test\/.

This seems rather inherent in the way the block works. In e2e tests, we could fix it by saving the post before publishing and viewing it, so that is already given the same slug as it would get upon publishing.

[ockham]

FWIW, here's the relevant block's code: https://github.com/godaddy-wordpress/coblocks/blob/0d644c66e96292baed08472631c0892b6b71e186/src/blocks/click-to-tweet/edit.js#L37-L43

[ockham]

Filed upstream issue: godaddy-wordpress/coblocks#1663

[fullofcaffeine]

@ockham Thanks for thoroughly documenting the issue as you worked towards fixing it ❤️

This looks good, let's 🚢 it!