mirror of
https://github.com/atom/atom.git
synced 2026-01-23 13:58:08 -05:00
Don't remove non accented character from history, improve test coverage
Unfortunately Chromium does not trigger a `compositionstart` before firing the text input event for the non accented character. Using `undo` to remove such character from the history is risky because it could be grouped with a previous change, thus making Atom undo too much. With this commit we simply keep the behavior master exhibits as of today. In the process of rewriting this code path, however, we fixed a bug that occurred when opening the accented character menu while holding another key, and improved test coverage as well by simulating the events the browser triggers.
This commit is contained in:
Antonio Scandurra
@@ -2400,6 +2400,202 @@ describe('TextEditorComponent', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('keyboard input', () => {
|
||||
it('handles inserted accented characters via the press-and-hold menu on macOS correctly', () => {
|
||||
const {editor, component, element} = buildComponent({text: ''})
|
||||
editor.insertText('x')
|
||||
editor.setCursorBufferPosition([0, 1])
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// then closing it via ESC.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didKeydown({code: 'Escape'})
|
||||
component.didKeyup({code: 'Escape'})
|
||||
expect(editor.getText()).toBe('xa')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xaa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// then selecting an alternative by typing a number.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didKeydown({code: 'Digit2'})
|
||||
component.didKeyup({code: 'Digit2'})
|
||||
component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xáa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// then selecting an alternative by clicking on it.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xáa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// cycling through the alternatives with the arrows, then selecting one of them with Enter.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionStart({data: ''})
|
||||
component.didCompositionUpdate({data: 'à'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xà')
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
component.didKeydown({code: 'Enter'})
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didCompositionEnd({data: 'á', target: component.refs.hiddenInput})
|
||||
component.didKeyup({code: 'Enter'})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xáa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// cycling through the alternatives with the arrows, then closing it via ESC.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionStart({data: ''})
|
||||
component.didCompositionUpdate({data: 'à'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xà')
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
component.didKeydown({code: 'Escape'})
|
||||
component.didCompositionUpdate({data: 'a'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didCompositionEnd({data: 'a', target: component.refs.hiddenInput})
|
||||
component.didKeyup({code: 'Escape'})
|
||||
expect(editor.getText()).toBe('xa')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xaa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate pressing the O key and holding the A key to open the press-and-hold menu right before releasing the O key,
|
||||
// cycling through the alternatives with the arrows, then closing it via ESC.
|
||||
component.didKeydown({code: 'KeyO'})
|
||||
component.didKeypress({code: 'KeyO'})
|
||||
component.didTextInput({data: 'o', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyO'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionStart({data: ''})
|
||||
component.didCompositionUpdate({data: 'à'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xoà')
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xoá')
|
||||
component.didKeydown({code: 'Escape'})
|
||||
component.didCompositionUpdate({data: 'a'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didCompositionEnd({data: 'a', target: component.refs.hiddenInput})
|
||||
component.didKeyup({code: 'Escape'})
|
||||
expect(editor.getText()).toBe('xoa')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
|
||||
// Simulate holding the A key to open the press-and-hold menu,
|
||||
// cycling through the alternatives with the arrows, then closing it by changing focus.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionStart({data: ''})
|
||||
component.didCompositionUpdate({data: 'à'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xà')
|
||||
component.didKeydown({code: 'ArrowRight'})
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didKeyup({code: 'ArrowRight'})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
component.didCompositionUpdate({data: 'á'})
|
||||
component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didCompositionEnd({data: 'á', target: component.refs.hiddenInput})
|
||||
expect(editor.getText()).toBe('xá')
|
||||
// Ensure another "a" can be typed correctly.
|
||||
component.didKeydown({code: 'KeyA'})
|
||||
component.didKeypress({code: 'KeyA'})
|
||||
component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}})
|
||||
component.didKeyup({code: 'KeyA'})
|
||||
expect(editor.getText()).toBe('xáa')
|
||||
editor.undo()
|
||||
expect(editor.getText()).toBe('x')
|
||||
})
|
||||
})
|
||||
|
||||
describe('styling changes', () => {
|
||||
it('updates the rendered content based on new measurements when the font dimensions change', async () => {
|
||||
const {component, element, editor} = buildComponent({rowsPerTile: 1, autoHeight: false})
|
||||
|
||||
@@ -1393,10 +1393,12 @@ class TextEditorComponent {
|
||||
this.compositionCheckpoint = null
|
||||
}
|
||||
|
||||
// Undo insertion of the original non-accented character so it is discarded
|
||||
// from the history and does not reappear on undo
|
||||
// If the input event is fired while the accented character menu is open it
|
||||
// means that the user has chosen one of the accented alternatives. Thus, we
|
||||
// will replace the original non accented character with the selected
|
||||
// alternative.
|
||||
if (this.accentedCharacterMenuIsOpen) {
|
||||
this.props.model.undo()
|
||||
this.props.model.selectLeft()
|
||||
}
|
||||
|
||||
this.props.model.insertText(event.data, {groupUndo: true})
|
||||
@@ -1413,24 +1415,24 @@ class TextEditorComponent {
|
||||
// before observing any keyup event, we observe events in the following
|
||||
// sequence:
|
||||
//
|
||||
// keydown(keyCode: X), keypress, keydown(keyCode: X)
|
||||
// keydown(code: X), keypress, keydown(code: X)
|
||||
//
|
||||
// The keyCode X must be the same in the keydown events that bracket the
|
||||
// The code X must be the same in the keydown events that bracket the
|
||||
// keypress, meaning we're *holding* the _same_ key we intially pressed.
|
||||
// Got that?
|
||||
didKeydown (event) {
|
||||
if (this.lastKeydownBeforeKeypress != null) {
|
||||
if (this.lastKeydownBeforeKeypress.keyCode === event.keyCode) {
|
||||
if (this.lastKeydownBeforeKeypress.code === event.code) {
|
||||
this.accentedCharacterMenuIsOpen = true
|
||||
this.props.model.selectLeft()
|
||||
}
|
||||
|
||||
this.lastKeydownBeforeKeypress = null
|
||||
} else {
|
||||
this.lastKeydown = event
|
||||
}
|
||||
|
||||
this.lastKeydown = event
|
||||
}
|
||||
|
||||
didKeypress () {
|
||||
didKeypress (event) {
|
||||
this.lastKeydownBeforeKeypress = this.lastKeydown
|
||||
this.lastKeydown = null
|
||||
|
||||
@@ -1439,9 +1441,11 @@ class TextEditorComponent {
|
||||
this.accentedCharacterMenuIsOpen = false
|
||||
}
|
||||
|
||||
didKeyup () {
|
||||
this.lastKeydownBeforeKeypress = null
|
||||
this.lastKeydown = null
|
||||
didKeyup (event) {
|
||||
if (this.lastKeydownBeforeKeypress && this.lastKeydownBeforeKeypress.code === event.code) {
|
||||
this.lastKeydownBeforeKeypress = null
|
||||
this.lastKeydown = null
|
||||
}
|
||||
}
|
||||
|
||||
// The IME composition events work like this:
|
||||
@@ -1451,13 +1455,14 @@ class TextEditorComponent {
|
||||
// 2. compositionupdate fired; event.data == 's'
|
||||
// User hits arrow keys to move around in completion helper
|
||||
// 3. compositionupdate fired; event.data == 's' for each arry key press
|
||||
// User escape to cancel
|
||||
// 4. compositionend fired
|
||||
// OR User chooses a completion
|
||||
// User escape to cancel OR User chooses a completion
|
||||
// 4. compositionend fired
|
||||
// 5. textInput fired; event.data == the completion string
|
||||
didCompositionStart () {
|
||||
this.compositionCheckpoint = this.props.model.createCheckpoint()
|
||||
if (this.accentedCharacterMenuIsOpen) {
|
||||
this.props.model.selectLeft()
|
||||
}
|
||||
}
|
||||
|
||||
didCompositionUpdate (event) {
|
||||
|
||||
Reference in New Issue
Block a user