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.

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

Revert shortcut change in block-deletion e2e test #9477

Merged
merged 2 commits into from Sep 3, 2018

Conversation

Projects
None yet
4 participants
@talldan
Contributor

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 Aug 30, 2018

@caxco93

This comment has been minimized.

Show comment
Hide comment
@caxco93

caxco93 Aug 30, 2018

Contributor

Is this addressing wrongfully failed e2e tests?

Contributor

caxco93 commented Aug 30, 2018

Is this addressing wrongfully failed e2e tests?

@talldan

This comment has been minimized.

Show comment
Hide comment
@talldan

talldan Aug 30, 2018

Contributor

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

Contributor

talldan commented Aug 30, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Aug 30, 2018

Contributor

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

Contributor

youknowriad commented Aug 30, 2018

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

@talldan

This comment has been minimized.

Show comment
Hide comment
@talldan

talldan Aug 30, 2018

Contributor

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

Contributor

talldan commented Aug 30, 2018

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

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Aug 30, 2018

Contributor

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 🤷‍♂️

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

This comment has been minimized.

Show comment
Hide comment
@talldan

talldan Aug 31, 2018

Contributor

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.

Contributor

talldan commented Aug 31, 2018

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

This comment has been minimized.

Show comment
Hide comment
@talldan

talldan Sep 3, 2018

Contributor

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

Contributor

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

LGTM 👍

@talldan talldan changed the title from Revert "Update shortcut for remove block in block-deletion test" to Revert shortcut change in block-deletion e2e test Sep 3, 2018

@talldan talldan merged commit cf74b67 into master Sep 3, 2018

2 checks passed

codecov/project 50.23% (-0.01%) compared to c5573b7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@talldan talldan deleted the revert/deletion-shortcut-in-e2e-test branch Sep 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment