Skip to content

Commit

Permalink
Fix an issue when call onKeyUp handler in PickerPlugin (#297)
Browse files Browse the repository at this point in the history
* Check isSuggesting for function key in PickerPlugin

* 7.3.2

* Fix emoji reappear issue when space after backspace
  • Loading branch information
JiuqingSong authored Apr 29, 2019
1 parent 6476fad commit 61bd201
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 15 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "roosterjs",
"version": "7.3.1",
"version": "7.3.2",
"description": "Framework-independent javascript editor",
"repository": {
"type": "git",
Expand Down
7 changes: 2 additions & 5 deletions packages/roosterjs-editor-core/lib/coreAPI/attachDomEvent.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import EditorCore, { AttachDomEvent } from '../interfaces/EditorCore';
import isModifierKey from '../eventApi/isModifierKey';
import isCharacterValue from '../eventApi/isCharacterValue';
import { PluginDomEvent, PluginEventType } from 'roosterjs-editor-types';

const attachDomEvent: AttachDomEvent = (
Expand All @@ -14,10 +14,7 @@ const attachDomEvent: AttachDomEvent = (
// event.key is longer than 1 for num pad input. But here we just want to improve performance as much as possible.
// So if we missed some case here it is still acceptable.
if (
(isKeyboardEvent(event) &&
!isModifierKey(event) &&
event.key &&
event.key.length == 1) ||
(isKeyboardEvent(event) && isCharacterValue(event)) ||
pluginEventType == PluginEventType.Input
) {
event.stopPropagation();
Expand Down
12 changes: 12 additions & 0 deletions packages/roosterjs-editor-core/lib/eventApi/isCharacterValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import isModifierKey from './isModifierKey';

/**
* Returns true when the event was fired from a key that produces a character value, otherwise false
* This detection is not 100% accurate. event.key is not fully supported by all browsers, and in some browsers (e.g. IE),
* event.key is longer than 1 for num pad input. But here we just want to improve performance as much as possible.
* So if we missed some case here it is still acceptable.
* @param event The keyboard event object
*/
export default function isCharacterValue(event: KeyboardEvent): boolean {
return !isModifierKey(event) && event.key && event.key.length == 1;
}
5 changes: 4 additions & 1 deletion packages/roosterjs-editor-core/lib/eventApi/isModifierKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ const CTRL_CHARCODE = 'Control';
const ALT_CHARCODE = 'Alt';
const META_CHARCODE = 'Meta';

// Returns true when the event was fired from a modifier key, otherwise false
/**
* Returns true when the event was fired from a modifier key, otherwise false
* @param event The keyboard event object
*/
export default function isModifierKey(event: KeyboardEvent): boolean {
const isCtrlKey = event.ctrlKey || event.key === CTRL_CHARCODE;
const isAltKey = event.altKey || event.key === ALT_CHARCODE;
Expand Down
1 change: 1 addition & 0 deletions packages/roosterjs-editor-core/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,4 @@ export {
} from './eventApi/cacheGetContentSearcher';
export { default as cacheGetElementAtCursor } from './eventApi/cacheGetElementAtCursor';
export { default as isModifierKey } from './eventApi/isModifierKey';
export { default as isCharacterValue } from './eventApi/isCharacterValue';
2 changes: 1 addition & 1 deletion packages/roosterjs-plugin-picker/lib/PickerDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export interface PickerDataProvider {
onIsSuggestingChanged: (isSuggesting: boolean) => void;

// Function called when the query string (text after the trigger symbol) is updated.
queryStringUpdated: (queryString: string) => void;
queryStringUpdated: (queryString: string, isExactMatch: boolean) => void;

// Function called when a keypress is issued that would "select" a currently highlighted option.
selectOption?: () => void;
Expand Down
33 changes: 26 additions & 7 deletions packages/roosterjs-plugin-picker/lib/PickerPlugin.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { Browser, createRange, PartialInlineElement } from 'roosterjs-editor-dom';
import { cacheGetContentSearcher, Editor, EditorPlugin } from 'roosterjs-editor-core';
import { isModifierKey } from 'roosterjs-editor-core';
import { PickerDataProvider, PickerPluginOptions } from './PickerDataProvider';
import { replaceWithNode } from 'roosterjs-editor-api';
import {
cacheGetContentSearcher,
Editor,
EditorPlugin,
isCharacterValue,
isModifierKey,
} from 'roosterjs-editor-core';
import {
NodePosition,
PluginKeyboardEvent,
Expand Down Expand Up @@ -128,6 +133,11 @@ export default class PickerPlugin<T extends PickerDataProvider = PickerDataProvi
event.source == ChangeSource.SetContent &&
this.dataProvider.onContentChanged
) {
// Stop suggesting since content is fully changed
if (this.isSuggesting) {
this.setIsSuggesting(false);
}

// Undos and other major changes to document content fire this type of event.
// Inform the data provider of the current picker placed elements in the body.
let elementIds: string[] = [];
Expand All @@ -148,7 +158,8 @@ export default class PickerPlugin<T extends PickerDataProvider = PickerDataProvi
if (
event.eventType == PluginEventType.KeyUp &&
!this.eventHandledOnKeyDown &&
!isModifierKey(event.rawEvent)
(isCharacterValue(event.rawEvent) ||
(!isModifierKey(event.rawEvent) && this.isSuggesting))
) {
this.onKeyUpDomEvent(event);
} else if (event.eventType == PluginEventType.MouseUp) {
Expand Down Expand Up @@ -231,7 +242,8 @@ export default class PickerPlugin<T extends PickerDataProvider = PickerDataProvi
if (this.isSuggesting) {
// Word before cursor represents the text prior to the cursor, up to and including the trigger symbol.
const wordBeforeCursor = this.getWord(event);
const trimmedWordBeforeCursor = wordBeforeCursor.substring(1).trim();
const wordBeforeCursorWithoutTriggerChar = wordBeforeCursor.substring(1);
const trimmedWordBeforeCursor = wordBeforeCursorWithoutTriggerChar.trim();

// If we hit a case where wordBeforeCursor is just the trigger character,
// that means we've gotten a onKeyUp event right after it's been typed.
Expand All @@ -246,7 +258,10 @@ export default class PickerPlugin<T extends PickerDataProvider = PickerDataProvi
trimmedWordBeforeCursor.length > 0 &&
trimmedWordBeforeCursor.split(' ').length <= 4)
) {
this.dataProvider.queryStringUpdated(trimmedWordBeforeCursor);
this.dataProvider.queryStringUpdated(
trimmedWordBeforeCursor,
wordBeforeCursorWithoutTriggerChar == trimmedWordBeforeCursor
);
this.setLastKnownRange(this.editor.getSelectionRange());
} else {
this.setIsSuggesting(false);
Expand All @@ -260,8 +275,12 @@ export default class PickerPlugin<T extends PickerDataProvider = PickerDataProvi
wordBeforeCursor[0] == this.pickerOptions.triggerCharacter
) {
this.setIsSuggesting(true);
let shortWord = wordBeforeCursor.substring(1).trim();
this.dataProvider.queryStringUpdated(shortWord);
const wordBeforeCursorWithoutTriggerChar = wordBeforeCursor.substring(1);
let trimmedWordBeforeCursor = wordBeforeCursorWithoutTriggerChar.trim();
this.dataProvider.queryStringUpdated(
trimmedWordBeforeCursor,
wordBeforeCursorWithoutTriggerChar == trimmedWordBeforeCursor
);
this.setLastKnownRange(this.editor.getSelectionRange());
if (this.dataProvider.setCursorPoint) {
// Determine the bounding rectangle for the @mention
Expand Down

0 comments on commit 61bd201

Please sign in to comment.