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.

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

[Tiny PR] Text patterns: fix undo after mouse move #18533

Merged
merged 1 commit into from Nov 18, 2019
Merged

Conversation

@ellatrix
Copy link
Member

ellatrix commented Nov 14, 2019

Description

Fixes #18492.

How has this been tested?

  1. Type 1. inside a paragraph. The block should convert to a list block.
  2. Move the mouse outside the block.
  3. Press Backspace. The block transform should be undone.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .
// Undoing an automatic change should still be possible after mouse
// move.
case 'STOP_TYPING':
return state;
}

// Reset the state by default (for any action not handled).

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 15, 2019

Contributor

I think the root problem here is that this is dangerous in reducers in general. I'm pretty sure this error can happen again if we introduce new random actions. That said, now we have an e2e test to be aware of it at least.

This comment has been minimized.

Copy link
@ellatrix

ellatrix Nov 15, 2019

Author Member

Right, a newly introduced action might need to be added here, but it's more likely that it shouldn't be added here. If we do the opposite, it might be that we do the wrong action on Backspace, which I think is worse. I think it's better to whitelist the actions that should be ignored (which there are way less of).

Copy link
Contributor

youknowriad left a comment

While this fixes the issue, the fact that the caret is not in the paragraph after the undo is disturbing but I guess this is a different issue.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 15, 2019

@youknowriad Right, there a separate PR for that: #17824. Looks like one e2e test needs to be fixed and needs a rebase, but is otherwise looking good. I'll try to fix it sometime soon.

@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 15, 2019

A transformation e2e tests seem to be consistently failing... and I have no idea why. It seems to be around the full site editing blocks. @jorgefilipecosta or @epiqueras Do you have any idea?

FAIL packages/e2e-tests/specs/experimental/block-transforms.test.js (383.667s)
997  ● Block transforms › should contain the expected transforms
998
999    expect(received).toEqual(expected)
1000
1001    Difference:
1002
1003    - Expected
1004    + Received
1005
1006    @@ -374,10 +374,22 @@
1007            "Preformatted",
1008            "Verse",
1009          ],
1010          "originalBlock": "Paragraph",
1011        },
1012    +   "core__post-content": Object {
1013    +     "availableTransforms": Array [
1014    +       "Group",
1015    +     ],
1016    +     "originalBlock": "Post Content",
1017    +   },
1018    +   "core__post-title": Object {
1019    +     "availableTransforms": Array [
1020    +       "Group",
1021    +     ],
1022    +     "originalBlock": "Post Title",
1023    +   },
1024        "core__preformatted": Object {
1025          "availableTransforms": Array [
1026            "Group",
1027            "Paragraph",
1028          ],
@jorgefilipecosta

This comment has been minimized.

Copy link
Member

jorgefilipecosta commented Nov 15, 2019

Hi @ellatrix, I'm submitting a fix.

@epiqueras

This comment has been minimized.

Copy link
Contributor

epiqueras commented Nov 15, 2019

Thank you @jorgefilipecosta !

Sorry, I read the failures too quickly and thought they were the unrelated group transform failures PRs always get.

@ellatrix ellatrix force-pushed the fix/pattern-undo-2 branch from 58ee6bc to 44c3017 Nov 18, 2019
@ellatrix

This comment has been minimized.

Copy link
Member Author

ellatrix commented Nov 18, 2019

Thanks for the review, @youknowriad!

@ellatrix ellatrix merged commit 6c95d81 into master Nov 18, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@ellatrix ellatrix deleted the fix/pattern-undo-2 branch Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.