Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

I/5826: Add a helper to handle custom Ctrl+A behaviour #16

Merged
merged 7 commits into from
Jan 13, 2020
Merged

Conversation

panr
Copy link
Contributor

@panr panr commented Jan 10, 2020

Suggested merge commit message (convention)

Fix: Pressing Ctrl+A when selection range is inside an exception selects text only within an exception in restricted editing. Second Ctrl+A selects entire text in the editor. Closes ckeditor/ckeditor5#5826.

@jodator jodator self-requested a review January 10, 2020 10:45
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

R-:

  • please use editor.keystrokes for adding keystrokes - we have a pattern for this. I don't know if it was designed this way but I can see a benefit of using keystroke handler in reading registered keystrokes in the future
  • other than that some small code style improvements ;)

src/restrictededitingmodeediting.js Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
tests/restrictededitingmodeediting.js Show resolved Hide resolved
@panr panr requested a review from jodator January 13, 2020 08:14
@panr
Copy link
Contributor Author

panr commented Jan 13, 2020

OK, I hope I fixed all issues. Also increased test coverage by +0.2% to hit 100% 😄

Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

Cosmetics only :)

  1. Leftover spy in tets.
  2. Different code style for building event handlers (without ...).

tests/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
tests/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
src/restrictededitingmodeediting.js Outdated Show resolved Hide resolved
@panr panr requested a review from jodator January 13, 2020 09:32
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@jodator jodator merged commit 569d588 into master Jan 13, 2020
@jodator jodator deleted the i/5826 branch January 13, 2020 09:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select editiable region data on CTRL+A
2 participants