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.

Sign up for GitHub

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

Improve openAllBlockInserterCategories function; #14460

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Improve openAllBlockInserterCategories function; #14460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/e2e-test-utils/src/open-all-block-inserter-categories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* Opens all block inserter categories.
*/
export async function openAllBlockInserterCategories() {
const notOpenCategoryPanels = await page.$$(
'.block-editor-inserter__results .components-panel__body:not(.is-opened)'
);
for ( const categoryPanel of notOpenCategoryPanels ) {
const notOppenedCategorySelector = '.block-editor-inserter__results .components-panel__body:not(.is-opened)';
let categoryPanel = await page.$( notOppenedCategorySelector );
while ( categoryPanel !== null ) {
await categoryPanel.click();
categoryPanel = await page.$( notOppenedCategorySelector );
Copy link
Member

Choose a reason for hiding this comment

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

How about we keep the for of loop and we wait until it has is-opened class applied? I never feel confident about using while loop :)
I'm not sure to be honest, maybe this is okay since it has to await and would timeout anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this approach would still be problematic, because if we used the selector '.block-editor-inserter__results .components-panel__body' in a for of, as soon as we applied the first click, all the other selectors would be invalid, because they reference an element that no longer exists on the dom because of a rerender that happens on click so we would not be able to use the element to wait for a class.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's a complex stuff :)

}
}
7 changes: 1 addition & 6 deletions packages/e2e-tests/specs/plugins/inner-blocks-allowed-blocks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ import {
openGlobalBlockInserter,
} from '@wordpress/e2e-test-utils';

// Todo: Understand why this test causing intermitent fails on travis.
// Error: ● Allowed Blocks Setting on InnerBlocks › allows all blocks if the allowed blocks setting was not set
// Node is detached from document
// at ElementHandle._scrollIntoViewIfNeeded (../../node_modules/puppeteer/lib/ElementHandle.js:75:13)
// eslint-disable-next-line jest/no-disabled-tests
describe.skip( 'Allowed Blocks Setting on InnerBlocks ', () => {
describe( 'Allowed Blocks Setting on InnerBlocks ', () => {
const paragraphSelector = '.block-editor-rich-text__editable.wp-block-paragraph';
beforeAll( async () => {
await activatePlugin( 'gutenberg-test-innerblocks-allowed-blocks' );
Expand Down