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

Jump to bottom

Disable the navigation mode when clicking the redirect element #17798

Merged
merged 2 commits into from
Oct 9, 2019
Merged

Disable the navigation mode when clicking the redirect element #17798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca40e24
Disable the navigation mode when clicking the redirect element
youknowriad Oct 7, 2019
0b1dc06
run the test in navigation mode
youknowriad Oct 9, 2019
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
1 change: 1 addition & 0 deletions packages/block-editor/src/components/writing-flow/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ class WritingFlow extends Component {
* Sets focus to the end of the last tabbable text field, if one exists.
*/
focusLastTextField() {
this.disableNavigationMode();
const focusableNodes = focus.focusable.find( this.container );
const target = findLast( focusableNodes, isTabbableTextField );
if ( target ) {
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e-tests/specs/adding-blocks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ describe( 'adding blocks', () => {
}

it( 'Should insert content using the placeholder and the regular inserter', async () => {
// This ensures the editor is loaded in navigation mode.
await page.reload();
Copy link
Member

Choose a reason for hiding this comment

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

Makes me wonder if there's any value in being able to disable it per test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

90% of our tests require it to be disabled. Initially, I tried disabling it per test but I noticed that I was doing it for almost all the tests.

Copy link
Member

Choose a reason for hiding this comment

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

I know, I meant having some sort of option to disable it when creating a new post for example. Maybe not worth it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right! I think it's a good thing to consider. Let's see how this gets affected by #17088 when implemented.


// Click below editor to focus last field (block appender)
await clickBelow( await page.$( '.block-editor-default-block-appender' ) );
expect( await page.$( '[data-type="core/paragraph"]' ) ).not.toBeNull();
Expand Down