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

Selection Management step 1 #247

Merged
merged 3 commits into from
Feb 28, 2019
Merged

Selection Management step 1 #247

merged 3 commits into from
Feb 28, 2019

Conversation

JiuqingSong
Copy link
Collaborator

I was planning to start the PR once I finished all steps, but it turns out that I still need some more time to finialize the other steps, and the first step can already fix the "paste hyperlink cause cursor pos wrong" issue, so let's start the first step PR.

The general idea of selection management is to cache a selection range every time a selection has been changed. In the first step, I split the core API select() into two steps:

  1. create selection range from several combinations of parameters (this part is merged to existing API createRange()):
    1.1 Single node => range around the node
    1.2 Two nodes => range around the two nodes
    1.3 Node+offset => collapsed range at the node+offset
    1.4 Node1+offset1+node2+offset2 => range start at node1+offset1, end at node2+offset2
    1.5 Node+startPath (number[]) + endPath (number[]) => range start at start path, end at end path
    1.6 Single NodePosition => collapsed range at the position
    1.7 Two NodePositions => range start at position 1, end at position 2
  2. select the range
    2.1 Rename the API to selectRange
    2.2 Add a parameter ''skipSameRange", when set to true, do nothing if new range is the same with existing one
    2.3 deprecate the old API select()

Then change the core API focus() to use selectRange() API with skipSameRange=true, to avoid lose current format state (B/I/U) when call focus().
Change all places where select() is called to new API.
The name and parameter list of Editor method Editor.select() are not changed,

let needAddRange = true;

if (selection.rangeCount > 0) {
// Workaround IE exception 800a025e
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the IE exception that selection has no rangeCount? Curious what then is the behavior in IE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be an IE bug.

@@ -517,7 +520,8 @@ export default class Editor {
): boolean;

public select(arg1: any, arg2?: any, arg3?: any, arg4?: any): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a duplicate of the const defined in selectRange?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the public editor API, while selectRange is a CoreAPI and can't be called outside editor.

) {
node = node.childNodes[offset];
} else {
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you always break out of the for loop once the conditional is not true, doesn't that mean you only ever check the index of 0? Why do you need a loop in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is existing code, moved from getSelectionPath.ts. It has been there for a while and run perfectly. So I believe we don't need to change it.

@JiuqingSong JiuqingSong merged commit 09ba3fd into master Feb 28, 2019
@JiuqingSong JiuqingSong deleted the jisong/selectionmangement1 branch February 28, 2019 06:11
JiuqingSong added a commit that referenced this pull request Feb 28, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix bad merge
JiuqingSong added a commit that referenced this pull request Mar 5, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix keyboard event issue (#251)
JiuqingSong added a commit that referenced this pull request Mar 14, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix keyboard event issue (#251)

* Improve hyperlink onLinkClick option (#255)

Allow Hyperlink onLinkClick to fall back to default handling to reduce redundant code.

* Bump version (#256)

* fix autolink bug (#263)

* Filter CTRL+Click for Hyperlink plugin to main button (#262)

* 7.2.5 (#265)

* Fix some perf issue and some other changes (#272)

* 45000

* 7.2.6
JiuqingSong added a commit that referenced this pull request Apr 29, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix keyboard event issue (#251)

* Improve hyperlink onLinkClick option (#255)

Allow Hyperlink onLinkClick to fall back to default handling to reduce redundant code.

* Bump version (#256)

* fix autolink bug (#263)

* Filter CTRL+Click for Hyperlink plugin to main button (#262)

* 7.2.5 (#265)

* Fix some perf issue and some other changes (#272)

* 45000

* 7.2.6

* init commit

* syntax error

* remove Input event so doesn't re-autocomplete

* StopPropagation for Input event (#276)

* Re-implemented the processList API. (#275)

* revert unneded changes and add check for char val

* ready for pr

* forgot 0 case again

* export from root

* undefined --> null, removes attribute instead of set as undefined

* Fix #279 (#281)

* take out unwanted change

* try that again

* 7.3.0 (#282)

* Add dark mode publish variant (#284)

* Fix #286 (#287)

* init commit

* add charcodes

* Fix #290 (#291)

* 7.3.1 (#294)

* 7.3.1-darkmode.0

* Fix an issue when call onKeyUp handler in PickerPlugin (#297)

* Check isSuggesting for function key in PickerPlugin

* 7.3.2

* Fix emoji reappear issue when space after backspace
JiuqingSong added a commit that referenced this pull request Jun 10, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix keyboard event issue (#251)

* Improve hyperlink onLinkClick option (#255)

Allow Hyperlink onLinkClick to fall back to default handling to reduce redundant code.

* Bump version (#256)

* fix autolink bug (#263)

* Filter CTRL+Click for Hyperlink plugin to main button (#262)

* 7.2.5 (#265)

* Fix some perf issue and some other changes (#272)

* 45000

* 7.2.6

* init commit

* syntax error

* remove Input event so doesn't re-autocomplete

* StopPropagation for Input event (#276)

* Re-implemented the processList API. (#275)

* revert unneded changes and add check for char val

* ready for pr

* forgot 0 case again

* export from root

* undefined --> null, removes attribute instead of set as undefined

* Fix #279 (#281)

* take out unwanted change

* try that again

* 7.3.0 (#282)

* Add dark mode publish variant (#284)

* Fix #286 (#287)

* init commit

* add charcodes

* Fix #290 (#291)

* 7.3.1 (#294)

* Fix an issue when call onKeyUp handler in PickerPlugin (#297)

* Check isSuggesting for function key in PickerPlugin

* 7.3.2

* Fix emoji reappear issue when space after backspace

* Export code to codepen from demo site (#289)

* init commit

* move order one down

* mouseUp before

* Add more comments to improve doc (#302)

* Improve doc

* Add more comments

* Fix cursor stuck at list issue (#303)

* Fix cursor stuck at list issue

* fix build

* Add getTextContent

* address comments for getTextContent

* Address comment

* change some element to use the class param

* remove comments

* 7.4.0 (#308)
JiuqingSong added a commit that referenced this pull request Jun 11, 2019
* 7.1.1

* 7.1.2 Fix #218 (#219)

* 7.1.2 Fix #218

* Fix typo

* Track content changed events and report up to PickerPluginDataProvider (#222)

* Refactor ReplaceWithNode to use the editor core API (#223)

* Address comments raised in #225 but not addressed (#227)

* Fix a js error in insertNode (#229)

* 7.2.0 (#230)

* Fix 4 issues (#234)

* Fix #74 (#236)

* Fix #74

* Fix #74

* address comments

* Replace deprecated TSLint extension with MSFT TSLint (#240)

* 7.2.1 (#243)

* stopPropagation for key inputs (#245)

* Make picker plugin generic (#246)

* Make PickerPlugin generic

* fix build

* Selection Management step 1 (#247)

* Selection management step 1: New core API selectRange

* Fix build

* fix comment

* Fix keyboard event issue (#251)

* Improve hyperlink onLinkClick option (#255)

Allow Hyperlink onLinkClick to fall back to default handling to reduce redundant code.

* Bump version (#256)

* fix autolink bug (#263)

* Filter CTRL+Click for Hyperlink plugin to main button (#262)

* 7.2.5 (#265)

* Fix some perf issue and some other changes (#272)

* 45000

* 7.2.6

* init commit

* syntax error

* remove Input event so doesn't re-autocomplete

* StopPropagation for Input event (#276)

* Re-implemented the processList API. (#275)

* revert unneded changes and add check for char val

* ready for pr

* forgot 0 case again

* export from root

* undefined --> null, removes attribute instead of set as undefined

* Fix #279 (#281)

* take out unwanted change

* try that again

* 7.3.0 (#282)

* Add dark mode publish variant (#284)

* Fix #286 (#287)

* init commit

* add charcodes

* Fix #290 (#291)

* 7.3.1 (#294)

* Fix an issue when call onKeyUp handler in PickerPlugin (#297)

* Check isSuggesting for function key in PickerPlugin

* 7.3.2

* Fix emoji reappear issue when space after backspace

* Export code to codepen from demo site (#289)

* init commit

* move order one down

* mouseUp before

* Add more comments to improve doc (#302)

* Improve doc

* Add more comments

* Fix cursor stuck at list issue (#303)

* Fix cursor stuck at list issue

* fix build

* Add getTextContent

* address comments for getTextContent

* Address comment

* change some element to use the class param

* remove comments

* 7.4.0 (#308)

* Revert #298 since it breaks IE (#310)

* revert #298

* 7.4.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants