From 9a9cfbd130289f3f7ff6e8e74976aa0b2f4e728a Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 13 Jun 2020 16:20:18 +0100 Subject: [PATCH] Mobile: Resolves #2595: Add undo/redo support --- .eslintignore | 1 + .gitignore | 1 + CliClient/tests/services_UndoRedoService.js | 86 ++++++++++++++ CliClient/tests/test-utils.js | 12 +- ReactNativeClient/lib/AsyncActionQueue.ts | 20 +++- .../lib/components/screen-header.js | 36 +++++- .../lib/components/screens/note.js | 111 ++++++++++++++++-- .../lib/services/UndoRedoService.ts | 103 ++++++++++++++++ 8 files changed, 353 insertions(+), 17 deletions(-) create mode 100644 CliClient/tests/services_UndoRedoService.js create mode 100644 ReactNativeClient/lib/services/UndoRedoService.ts diff --git a/.eslintignore b/.eslintignore index b03cad12e..7d9f80633 100644 --- a/.eslintignore +++ b/.eslintignore @@ -115,6 +115,7 @@ ReactNativeClient/lib/services/keychain/KeychainServiceDriver.node.js ReactNativeClient/lib/services/keychain/KeychainServiceDriverBase.js ReactNativeClient/lib/services/ResourceEditWatcher.js ReactNativeClient/lib/services/SettingUtils.js +ReactNativeClient/lib/services/UndoRedoService.js ReactNativeClient/lib/ShareExtension.js ReactNativeClient/lib/shareHandler.js ReactNativeClient/PluginAssetsLoader.js diff --git a/.gitignore b/.gitignore index 293ad87e9..b75d64720 100644 --- a/.gitignore +++ b/.gitignore @@ -105,6 +105,7 @@ ReactNativeClient/lib/services/keychain/KeychainServiceDriver.node.js ReactNativeClient/lib/services/keychain/KeychainServiceDriverBase.js ReactNativeClient/lib/services/ResourceEditWatcher.js ReactNativeClient/lib/services/SettingUtils.js +ReactNativeClient/lib/services/UndoRedoService.js ReactNativeClient/lib/ShareExtension.js ReactNativeClient/lib/shareHandler.js ReactNativeClient/PluginAssetsLoader.js diff --git a/CliClient/tests/services_UndoRedoService.js b/CliClient/tests/services_UndoRedoService.js new file mode 100644 index 000000000..ae8ca5935 --- /dev/null +++ b/CliClient/tests/services_UndoRedoService.js @@ -0,0 +1,86 @@ +// /* eslint-disable no-unused-vars */ + +// require('app-module-path').addPath(__dirname); + +// const { asyncTest, fileContentEqual, setupDatabase, checkThrow, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('test-utils.js'); +// const KvStore = require('lib/services/KvStore.js'); +// const UndoRedoService = require('lib/services/UndoRedoService.js').default; + +// process.on('unhandledRejection', (reason, p) => { +// console.log('Unhandled Rejection at: Promise', p, 'reason:', reason); +// }); + +// jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; + +// describe('services_UndoRedoService', function() { + +// beforeEach(async (done) => { +// await setupDatabaseAndSynchronizer(1); +// await switchClient(1); +// done(); +// }); + +// it('should undo and redo', asyncTest(async () => { +// const service = new UndoRedoService(); + +// expect(service.canUndo).toBe(false); +// expect(service.canRedo).toBe(false); + +// service.push('test'); +// expect(service.canUndo).toBe(true); +// expect(service.canRedo).toBe(false); +// service.push('test 2'); +// service.push('test 3'); + +// expect(service.undo()).toBe('test 3'); +// expect(service.canRedo).toBe(true); +// expect(service.undo()).toBe('test 2'); +// expect(service.undo()).toBe('test'); + +// expect(checkThrow(() => service.undo())).toBe(true); + +// expect(service.canUndo).toBe(false); +// expect(service.canRedo).toBe(true); + +// expect(service.redo()).toBe('test'); +// expect(service.canUndo).toBe(true); +// expect(service.redo()).toBe('test 2'); +// expect(service.redo()).toBe('test 3'); + +// expect(service.canRedo).toBe(false); + +// expect(checkThrow(() => service.redo())).toBe(true); +// })); + +// it('should clear the redo stack when undoing', asyncTest(async () => { +// const service = new UndoRedoService(); + +// service.push('test'); +// service.push('test 2'); +// service.push('test 3'); + +// service.undo(); +// expect(service.canRedo).toBe(true); + +// service.push('test 4'); +// expect(service.canRedo).toBe(false); + +// expect(service.undo()).toBe('test 4'); +// expect(service.undo()).toBe('test 2'); +// })); + +// it('should limit the size of the undo stack', asyncTest(async () => { +// const service = new UndoRedoService(); + +// for (let i = 0; i < 30; i++) { +// service.push(`test${i}`); +// } + +// for (let i = 0; i < 20; i++) { +// service.undo(); +// } + +// expect(service.canUndo).toBe(false); +// })); + +// }); diff --git a/CliClient/tests/test-utils.js b/CliClient/tests/test-utils.js index 946437e81..c1450e1eb 100644 --- a/CliClient/tests/test-utils.js +++ b/CliClient/tests/test-utils.js @@ -378,6 +378,16 @@ async function checkThrowAsync(asyncFn) { return hasThrown; } +function checkThrow(fn) { + let hasThrown = false; + try { + fn(); + } catch (error) { + hasThrown = true; + } + return hasThrown; +} + function fileContentEqual(path1, path2) { const fs = require('fs-extra'); const content1 = fs.readFileSync(path1, 'base64'); @@ -563,4 +573,4 @@ class TestApp extends BaseApplication { } } -module.exports = { kvStore, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp }; +module.exports = { kvStore, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, checkThrow, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp }; diff --git a/ReactNativeClient/lib/AsyncActionQueue.ts b/ReactNativeClient/lib/AsyncActionQueue.ts index fc0592697..a958a3b1b 100644 --- a/ReactNativeClient/lib/AsyncActionQueue.ts +++ b/ReactNativeClient/lib/AsyncActionQueue.ts @@ -7,6 +7,11 @@ export interface QueueItem { context: any, } +export enum IntervalType { + Debounce = 1, + Fixed = 2, +} + // The AsyncActionQueue can be used to debounce asynchronous actions, to make sure // they run in the right order, and also to ensure that if multiple actions are emitted // only the last one is executed. This is particularly useful to save data in the background. @@ -15,12 +20,14 @@ export default class AsyncActionQueue { queue_:QueueItem[] = []; interval_:number; + intervalType_:number; scheduleProcessingIID_:any = null; processing_ = false; needProcessing_ = false; - constructor(interval:number = 100) { + constructor(interval:number = 100, intervalType:IntervalType = IntervalType.Debounce) { this.interval_ = interval; + this.intervalType_ = intervalType; } push(action:QueueItemAction, context:any = null) { @@ -39,6 +46,7 @@ export default class AsyncActionQueue { if (interval === null) interval = this.interval_; if (this.scheduleProcessingIID_) { + if (this.intervalType_ === IntervalType.Fixed) return; clearTimeout(this.scheduleProcessingIID_); } @@ -67,6 +75,16 @@ export default class AsyncActionQueue { this.processing_ = false; } + async reset() { + if (this.scheduleProcessingIID_) { + clearTimeout(this.scheduleProcessingIID_); + this.scheduleProcessingIID_ = null; + } + + this.queue_ = []; + return this.waitForAllDone(); + } + // Currently waitForAllDone() already finishes all the actions // as quickly as possible so we can make it an alias. async processAllNow() { diff --git a/ReactNativeClient/lib/components/screen-header.js b/ReactNativeClient/lib/components/screen-header.js index 151243b38..545fcd294 100644 --- a/ReactNativeClient/lib/components/screen-header.js +++ b/ReactNativeClient/lib/components/screen-header.js @@ -61,8 +61,8 @@ class ScreenHeaderComponent extends React.PureComponent { iconButton: { flex: 1, backgroundColor: theme.backgroundColor2, - paddingLeft: 15, - paddingRight: 15, + paddingLeft: 10, + paddingRight: 10, paddingTop: PADDING_V, paddingBottom: PADDING_V, }, @@ -248,6 +248,36 @@ class ScreenHeaderComponent extends React.PureComponent { ); } + const renderTopButton = (options) => { + if (!options.visible) return null; + + const icon = ; + const viewStyle = options.disabled ? this.styles().iconButtonDisabled : this.styles().iconButton; + + return ( + + {icon} + + ); + }; + + const renderUndoButton = () => { + return renderTopButton({ + iconName: 'md-undo', + onPress: this.props.onUndoButtonPress, + visible: this.props.showUndoButton, + disabled: this.props.undoButtonDisabled, + }); + }; + + const renderRedoButton = () => { + return renderTopButton({ + iconName: 'md-redo', + onPress: this.props.onRedoButtonPress, + visible: this.props.showRedoButton, + }); + }; + function selectAllButton(styles, onPress) { return ( @@ -462,6 +492,8 @@ class ScreenHeaderComponent extends React.PureComponent { {sideMenuComp} {backButtonComp} + {renderUndoButton(this.styles())} + {renderRedoButton(this.styles())} {saveButton( this.styles(), () => { diff --git a/ReactNativeClient/lib/components/screens/note.js b/ReactNativeClient/lib/components/screens/note.js index afd9cab1e..e25d0d566 100644 --- a/ReactNativeClient/lib/components/screens/note.js +++ b/ReactNativeClient/lib/components/screens/note.js @@ -8,6 +8,7 @@ const { uuid } = require('lib/uuid.js'); const { MarkdownEditor } = require('../../../MarkdownEditor/index.js'); const RNFS = require('react-native-fs'); const Note = require('lib/models/Note.js'); +const UndoRedoService = require('lib/services/UndoRedoService.js').default; const BaseItem = require('lib/models/BaseItem.js'); const Setting = require('lib/models/Setting.js'); const Resource = require('lib/models/Resource.js'); @@ -70,9 +71,14 @@ class NoteScreenComponent extends BaseScreenComponent { // margin. This forces RN to update the text input and to display it. Maybe that hack can be removed once RN is upgraded. // See https://github.com/laurent22/joplin/issues/1057 HACK_webviewLoadingState: 0, - }; - this.selection = null; + undoRedoButtonState: { + canUndo: false, + canRedo: false, + }, + + selection: { start: 0, end: 0 }, + }; this.saveActionQueues_ = {}; @@ -122,6 +128,8 @@ class NoteScreenComponent extends BaseScreenComponent { mode: 'view', }); + await this.undoRedoService_.reset(); + return true; } @@ -216,6 +224,39 @@ class NoteScreenComponent extends BaseScreenComponent { this.todoCheckbox_change = this.todoCheckbox_change.bind(this); this.titleTextInput_contentSizeChange = this.titleTextInput_contentSizeChange.bind(this); this.title_changeText = this.title_changeText.bind(this); + this.undoRedoService_stackChange = this.undoRedoService_stackChange.bind(this); + this.screenHeader_undoButtonPress = this.screenHeader_undoButtonPress.bind(this); + this.screenHeader_redoButtonPress = this.screenHeader_redoButtonPress.bind(this); + this.body_selectionChange = this.body_selectionChange.bind(this); + } + + undoRedoService_stackChange() { + this.setState({ undoRedoButtonState: { + canUndo: this.undoRedoService_.canUndo, + canRedo: this.undoRedoService_.canRedo, + } }); + } + + async undoRedo(type) { + const undoState = await this.undoRedoService_[type](this.undoState()); + if (!undoState) return; + + this.setState((state) => { + const newNote = Object.assign({}, state.note); + newNote.body = undoState.body; + return { + note: newNote, + selection: Object.assign({}, undoState.selection), + }; + }); + } + + screenHeader_undoButtonPress() { + this.undoRedo('undo'); + } + + screenHeader_redoButtonPress() { + this.undoRedo('redo'); } styles() { @@ -307,9 +348,14 @@ class NoteScreenComponent extends BaseScreenComponent { return shared.isModified(this); } - async UNSAFE_componentWillMount() { - this.selection = null; + undoState(noteBody = null) { + return { + body: noteBody === null ? this.state.note.body : noteBody, + selection: Object.assign({}, this.state.selection), + }; + } + async componentDidMount() { BackButtonService.addHandler(this.backHandler); NavService.addHandler(this.navHandler); @@ -318,6 +364,9 @@ class NoteScreenComponent extends BaseScreenComponent { await shared.initState(this); + this.undoRedoService_ = new UndoRedoService(); + this.undoRedoService_.on('stackChange', this.undoRedoService_stackChange); + if (this.state.note && this.state.note.body && Setting.value('sync.resourceDownloadMode') === 'auto') { const resourceIds = await Note.linkedResourceIds(this.state.note.body); await ResourceFetcher.instance().markForDownload(resourceIds); @@ -353,6 +402,8 @@ class NoteScreenComponent extends BaseScreenComponent { } this.saveActionQueue(this.state.note.id).processAllNow(); + + this.undoRedoService_.off('stackChange', this.undoRedoService_stackChange); } title_changeText(text) { @@ -362,10 +413,19 @@ class NoteScreenComponent extends BaseScreenComponent { } body_changeText(text) { + if (!this.undoRedoService_.canUndo) { + this.undoRedoService_.push(this.undoState()); + } else { + this.undoRedoService_.schedulePush(this.undoState()); + } shared.noteComponent_change(this, 'body', text); this.scheduleSave(); } + body_selectionChange(event) { + this.setState({ selection: event.nativeEvent.selection }); + } + makeSaveAction() { return async () => { return shared.saveNoteButton_press(this); @@ -584,9 +644,9 @@ class NoteScreenComponent extends BaseScreenComponent { const newNote = Object.assign({}, this.state.note); - if (this.state.mode == 'edit' && !Setting.value('editor.beta') && !!this.selection) { - const prefix = newNote.body.substring(0, this.selection.start); - const suffix = newNote.body.substring(this.selection.end); + if (this.state.mode == 'edit' && !Setting.value('editor.beta') && !!this.state.selection) { + const prefix = newNote.body.substring(0, this.state.selection.start); + const suffix = newNote.body.substring(this.state.selection.end); newNote.body = `${prefix}\n${resourceTag}\n${suffix}`; } else { newNote.body += `\n${resourceTag}`; @@ -1022,9 +1082,8 @@ class NoteScreenComponent extends BaseScreenComponent { multiline={true} value={note.body} onChangeText={(text) => this.body_changeText(text)} - onSelectionChange={({ nativeEvent: { selection } }) => { - this.selection = selection; - }} + selection={this.state.selection} + onSelectionChange={this.body_selectionChange} blurOnSubmit={false} selectionColor={theme.textSelectionColor} keyboardAppearance={theme.keyboardAppearance} @@ -1056,7 +1115,7 @@ class NoteScreenComponent extends BaseScreenComponent { // Save button is not really needed anymore with the improved save logic const showSaveButton = false; // this.state.mode == 'edit' || this.isModified() || this.saveButtonHasBeenShown_; - const saveButtonDisabled = !this.isModified(); + const saveButtonDisabled = true;// !this.isModified(); if (showSaveButton) this.saveButtonHasBeenShown_ = true; @@ -1067,7 +1126,20 @@ class NoteScreenComponent extends BaseScreenComponent { const titleComp = ( {isTodo && } - + ); @@ -1075,7 +1147,20 @@ class NoteScreenComponent extends BaseScreenComponent { return ( - + {titleComp} {bodyComponent} {!Setting.value('editor.beta') && actionButtonComp} diff --git a/ReactNativeClient/lib/services/UndoRedoService.ts b/ReactNativeClient/lib/services/UndoRedoService.ts new file mode 100644 index 000000000..f8b298d11 --- /dev/null +++ b/ReactNativeClient/lib/services/UndoRedoService.ts @@ -0,0 +1,103 @@ +import AsyncActionQueue from '../AsyncActionQueue'; +const EventEmitter = require('events'); + +class UndoQueue { + + private inner_:any[] = []; + private size_:number = 20; + + pop() { + return this.inner_.pop(); + } + + push(e:any) { + this.inner_.push(e); + while (this.length > this.size_) { + this.inner_.splice(0,1); + } + } + + get length():number { + return this.inner_.length; + } + + at(index:number):any { + return this.inner_[index]; + } + +} + +export default class UndoRedoService { + + private pushAsyncQueue:AsyncActionQueue = new AsyncActionQueue(700); + private undoStates:UndoQueue = new UndoQueue(); + private redoStates:UndoQueue = new UndoQueue(); + private eventEmitter:any = new EventEmitter(); + private isUndoing:boolean = false; + + constructor() { + this.push = this.push.bind(this); + } + + on(eventName:string, callback:Function) { + return this.eventEmitter.on(eventName, callback); + } + + off(eventName:string, callback:Function) { + return this.eventEmitter.removeListener(eventName, callback); + } + + push(state:any) { + this.undoStates.push(state); + this.redoStates = new UndoQueue(); + this.eventEmitter.emit('stackChange'); + } + + schedulePush(state:any) { + this.pushAsyncQueue.push(async () => { + this.push(state); + }); + } + + async undo(redoState:any) { + if (this.isUndoing) return; + if (!this.canUndo) throw new Error('Nothing to undo'); + this.isUndoing = true; + await this.pushAsyncQueue.processAllNow(); + const state = this.undoStates.pop(); + this.redoStates.push(redoState); + this.eventEmitter.emit('stackChange'); + this.isUndoing = false; + return state; + } + + async redo(undoState:any) { + if (this.isUndoing) return; + if (!this.canRedo) throw new Error('Nothing to redo'); + this.isUndoing = true; + await this.pushAsyncQueue.processAllNow(); + const state = this.redoStates.pop(); + this.undoStates.push(undoState); + this.eventEmitter.emit('stackChange'); + this.isUndoing = false; + return state; + } + + async reset() { + this.undoStates = new UndoQueue(); + this.redoStates = new UndoQueue(); + this.isUndoing = false; + const output = this.pushAsyncQueue.reset(); + this.eventEmitter.emit('stackChange'); + return output; + } + + get canUndo():boolean { + return !!this.undoStates.length; + } + + get canRedo():boolean { + return !!this.redoStates.length; + } + +}