-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Move caret to the end of pasted content #21755
Conversation
Size Change: +64 B (0%) Total Size: 816 kB
ℹ️ View Unchanged
|
This fix is so anticipated, thanks for working on it 👍 There are two unit tests to fix, see https://travis-ci.com/github/WordPress/gutenberg/jobs/321645821 for more details. It would be a great follow-up task to add e2e test that covers this fix as another PR. |
7884c55
to
2e51bda
Compare
@gziolo All checks are green now. Agreed about the e2e tests - will look into it once this one lands. |
Could an e2e test be added as part of this PR, please? Otherwise it will be easy for this to regress with selection and rich text refactorings. |
typeof action.initialPosition === 'number' | ||
) { | ||
return action.initialPosition; | ||
} else if ( action.type === 'SELECT_BLOCK' ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the type check needed above, but not here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds a numeric initialPosition to the REPLACE_BLOCKS
action in some circumstances only which means it may or may not be present. It doesn't do anything to SELECT_BLOCK
action so I left that code branch untouched - I assume there's always initialPosition
available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: you could consolidate those two if
statements into one that uses ||
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you insist I will, but I think it's more readable like that with a dedicated branch for each action :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I respect your preference, if ESLint doesn’t complain it’s fine as is 😃
packages/e2e-tests/specs/editor/various/multi-block-selection.test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just tested it. It feels amazing. Small but impactful change 💯
Edit: feel free to merge after rebase. It looks like there are now some conflicting changes in one of the files.
fd322c7
to
5240e71
Compare
Description
When you paste a couple paragraphs into Gutenberg, your cursor remains at the beginning of the last paragraph you pasted. I expect it to be at the end of the last paragraph pasted. This PR updates initialPosition to -1 which, in turn, moves the caret to the end of the paragraph.
Solves #5317
How has this been tested?
Tested locally:
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: