Skip to content

Migrate post editor feature preferences to use new preferences package #39115

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

Merged
talldan merged 4 commits into from
Mar 2, 2022
Merged

Migrate post editor feature preferences to use new preferences package #39115

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f73a32
Migrate post editor preferences to use new preferences package
talldan Feb 28, 2022
c79bdba
Fix css classes used in e2e tests
talldan Feb 28, 2022
a348603
Remove outdated note
talldan Mar 2, 2022
a0c0085
Re-alphabetize imports
talldan Mar 2, 2022
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
Prev Previous commit
Next Next commit
Fix css classes used in e2e tests
  • Loading branch information
@talldan
talldan committed Feb 28, 2022
commit c79bdbafcfb9d6a2bdf0bf5002f8642e4fa25e2d
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/click-on-more-menu-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { toggleMoreMenu } from './toggle-more-menu';

const SELECTORS = {
postEditorMenuContainer:
'//*[contains(concat(" ", @class, " "), " interface-more-menu__content ")]',
'//*[contains(concat(" ", @class, " "), " interface-more-menu-dropdown__content ")]',
Copy link
Member

@noisysocks noisysocks Mar 2, 2022

Choose a reason for hiding this comment

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

Can we target using something that's user oriented e.g. ARIA label?

Copy link
Contributor Author

@talldan talldan Mar 2, 2022

Choose a reason for hiding this comment

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

Could be tricky as the block toolbar has an identical label for its settings menu.

I would like to get rid of this logic that uses a different class name for the site editor:

gutenberg/packages/e2e-test-utils/src/click-on-more-menu-item.js

Lines 15 to 20 in 11ae04c

const SELECTORS = {
postEditorMenuContainer:
'//*[contains(concat(" ", @class, " "), " edit-post-more-menu__content ")]',
siteEditorMenuContainer:
'//*[contains(concat(" ", @class, " "), " edit-site-more-menu__content ")]',
};

I'll be able to remove that once I've done the migration for the site editor, so at that point I'll loop back and improve this.

Copy link
Contributor

@tellthemachines tellthemachines Mar 2, 2022

Choose a reason for hiding this comment

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

In my experience user-facing labels change more often than classnames 😅
It's always a compromise with e2e selectors; it's not a huge deal if we have to change them once in a while. Classnames also tend to be more unique than aria-labels.

siteEditorMenuContainer:
'//*[contains(concat(" ", @class, " "), " edit-site-more-menu__content ")]',
};
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/toggle-more-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const SELECTORS = {
postEditorMenuContent: '.interface-more-menu-dropdown__content',
siteEditorMenuContent: '.edit-site-more-menu__content',
postEditorMenu: '.interface-more-menu [aria-label="Options"]',
postEditorMenu: '.interface-more-menu-dropdown [aria-label="Options"]',
siteEditorMenu: '.edit-site-more-menu [aria-label="More tools & options"]',
};

Expand Down
6 changes: 3 additions & 3 deletions packages/e2e-tests/specs/editor/various/popovers.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ describe( 'popovers', () => {
describe( 'dropdown', () => {
it( 'toggles via click', async () => {
const isMoreMenuOpen = async () =>
!! ( await page.$( '.interface-more-menu__content' ) );
!! ( await page.$( '.interface-more-menu-dropdown__content' ) );
Copy link
Member

@noisysocks noisysocks Mar 2, 2022

Choose a reason for hiding this comment

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

Same again here. Could be a good opportunity to make these tests less dependent on implementation details such as class name.


expect( await isMoreMenuOpen() ).toBe( false );

// Toggle opened.
await page.click( '.interface-more-menu > button' );
await page.click( '.interface-more-menu-dropdown > button' );
expect( await isMoreMenuOpen() ).toBe( true );

// Toggle closed.
await page.click( '.interface-more-menu > button' );
await page.click( '.interface-more-menu-dropdown > button' );
expect( await isMoreMenuOpen() ).toBe( false );
} );
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-post/src/components/header/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
padding: 0 #{ $grid-unit-15 * 0.5 };
}

.interface-more-menu .components-button,
.interface-more-menu-dropdown .components-button,
.interface-pinned-items .components-button {
margin-right: 0;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/nux/src/components/dot-tip/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ $dot-scale: 3; // How much the pulse animation should scale up by in size
margin-right: 0;
}

&.components-popover.interface-more-menu__content:not([data-y-axis="middle"])[data-y-axis="right"] .components-popover__content {
&.components-popover.interface-more-menu-dropdown__content:not([data-y-axis="middle"])[data-y-axis="right"] .components-popover__content {
/*!rtl:ignore*/
margin-left: -12px;
}

&.components-popover.interface-more-menu__content:not([data-y-axis="middle"])[data-y-axis="left"] .components-popover__content {
&.components-popover.interface-more-menu-dropdown__content:not([data-y-axis="middle"])[data-y-axis="left"] .components-popover__content {
/*!rtl:ignore*/
margin-right: -12px;
}
Expand Down