Skip to content

Revert shortcut change in block-deletion e2e test#9477

Merged
talldan merged 2 commits intomasterfrom
revert/deletion-shortcut-in-e2e-test
Sep 3, 2018
Merged

Revert shortcut change in block-deletion e2e test#9477
talldan merged 2 commits intomasterfrom
revert/deletion-shortcut-in-e2e-test

Conversation

@talldan
Copy link
Copy Markdown
Contributor

@talldan talldan commented Aug 30, 2018

This reverts commit ef01a14.

An e2e test that uses the block deletion shortcut was merged in #8961. Unfortunately the shortcut key was reverted back. This PR also reverts the shortcut used in the test.

@talldan talldan added this to the 3.7 milestone Aug 30, 2018
@talldan talldan self-assigned this Aug 30, 2018
@talldan talldan requested a review from youknowriad August 30, 2018 16:09
@talldan talldan mentioned this pull request Aug 30, 2018
Merged
@caxco93
Copy link
Copy Markdown
Contributor

caxco93 commented Aug 30, 2018

Is this addressing wrongfully failed e2e tests?

@talldan
Copy link
Copy Markdown
Contributor Author

talldan commented Aug 30, 2018 •
edited
Loading

@caxco93 not wrongfully failed - I reverted a change to a shortcut, but missed reverting it in a test in a different PR. This PR fixes.

@youknowriad
Copy link
Copy Markdown
Contributor

youknowriad commented Aug 30, 2018

I wonder why it wasn't failing in my PR with "shift" instead of "alt"

@talldan
Copy link
Copy Markdown
Contributor Author

talldan commented Aug 30, 2018

I think it gets picked up by our event handlers as just a standalone BACKSPACE.

@youknowriad
Copy link
Copy Markdown
Contributor

youknowriad commented Aug 30, 2018

I guess the difference, is we'll have an empty paragraph in one case and remove the paragraph in the other case? I wonder if there's a way to update the test to make it break with shift 🤷‍♂️

@talldan
Copy link
Copy Markdown
Contributor Author

talldan commented Aug 31, 2018 •
edited
Loading

Yeah, I agree, that would be a good improvement. Making the test type some text before the backspace should have that effect.

I can push a commit that does that a bit later.

@gziolo gziolo modified the milestones: 3.7, 3.8 Aug 31, 2018
@talldan
Copy link
Copy Markdown
Contributor Author

talldan commented Sep 3, 2018

Hey @youknowriad - I've modified the test so that it'll fail now with the shortcut currently in master.
865e9ee

youknowriad
youknowriad approved these changes Sep 3, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@talldan talldan changed the title Revert "Update shortcut for remove block in block-deletion test" Revert shortcut change in block-deletion e2e test Sep 3, 2018
@talldan talldan merged commit cf74b67 into master Sep 3, 2018
@talldan talldan deleted the revert/deletion-shortcut-in-e2e-test branch September 3, 2018 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowriad youknowriad youknowriad approved these changes

Assignees

@talldan talldan

Labels

None yet

Projects

None yet

Milestone

3.8

Development

Successfully merging this pull request may close these issues.

4 participants

@talldan @caxco93 @youknowriad @gziolo