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.

Sign up for GitHub

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

[Rewrite Branch] Fixes undo redo selectall #2181

Merged
merged 7 commits into from
Aug 18, 2020
Merged

[Rewrite Branch] Fixes undo redo selectall #2181

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
63ef0c0
Fix SelectAll, Undo, Redo
belcherj Aug 17, 2020
c4b5e76
Fix electron checks
belcherj Aug 17, 2020
6d81a9e
Wire up editor command
belcherj Aug 17, 2020
fc99cc3
Create editCommandSender
belcherj Aug 18, 2020
e389160
Update function names
belcherj Aug 18, 2020
5da2edc
Fix command sender
belcherj Aug 18, 2020
c11489b
Add editorCommand to types
belcherj Aug 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions desktop/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const importNotes = require('./evernote-import');
const platform = require('./detect/platform');
const updater = require('./updater');
const { isDev } = require('./env');
const spellcheck = require('./spellchecker');
const contextMenu = require('./context-menu');

require('module').globalPaths.push(path.resolve(path.join(__dirname)));

Expand Down Expand Up @@ -76,7 +76,7 @@ module.exports = function main() {
mainWindow.loadUrl(url);
}

spellcheck(mainWindow);
contextMenu(mainWindow);

if (
'test' !== process.env.NODE_ENV &&
Expand Down
45 changes: 45 additions & 0 deletions desktop/context-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

/**
* External dependencies
*/
const { Menu } = require('electron');

const { appCommandSender } = require('../menus/utils');

module.exports = function (mainWindow) {
mainWindow.webContents.on('context-menu', (event, params) => {
const { editFlags } = params;
const template = [
{
id: 'selectAll',
label: 'Select All',
click: appCommandSender('editorCommand', { action: 'selectAll' }),
enabled: editFlags.canSelectAll,
},
{
id: 'cut',
label: 'Cut',
role: 'cut',
enabled: editFlags.canCut,
},
{
id: 'copy',
label: 'Copy',
role: 'copy',
enabled: editFlags.canCopy,
},
{
id: 'paste',
label: 'Paste',
role: 'paste',
enabled: editFlags.canPaste,
},
{
type: 'separator',
},
];
const menu = Menu.buildFromTemplate(template);
menu.popup({});
Copy link
Member

@dmsnell dmsnell Aug 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I assume these are split up as part of the development process? if we find it useful we could inline the function. how could it be useful? if we found more value in the brevity than in the verbosity

mainWindow.webContents.on('context-menu', (event, { editFlags }) =>
	Menu.buildFromTemplate([
		{
			…
		},
		…
	]).popup({});

I'm assuming that argument-destructuring works since the object destructuring is working.
if you don't think this is a positive change just ignore it.

});
};
12 changes: 6 additions & 6 deletions desktop/menus/edit-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ const buildEditMenu = (settings, isAuthenticated) => {
submenu: [
{
label: '&Undo',
role: 'undo',
click: appCommandSender('editorCommand', { action: 'undo' }),
},
{
label: '&Redo',
role: 'redo',
click: appCommandSender('editorCommand', { action: 'redo' }),
},
{
type: 'separator',
Expand All @@ -32,27 +32,27 @@ const buildEditMenu = (settings, isAuthenticated) => {
},
{
label: '&Select All',
role: 'selectall',
click: appCommandSender('editorCommand', { action: 'selectAll' }),
},
{ type: 'separator' },
{
label: '&Trash Note',
visible: isAuthenticated,
click: appCommandSender({ action: 'trashNote' }),
click: appCommandSender('appCommnad', { action: 'trashNote' }),
},
{ type: 'separator' },
{
label: 'Search &Notes…',
visible: isAuthenticated,
click: appCommandSender({ action: 'focusSearchField' }),
click: appCommandSender('appCommnad', { action: 'focusSearchField' }),
},
{ type: 'separator' },
{
label: 'C&heck Spelling',
visible: isAuthenticated,
type: 'checkbox',
checked: settings.spellCheckEnabled,
click: appCommandSender({ action: 'toggleSpellCheck' }),
click: appCommandSender('appCommnad', { action: 'toggleSpellCheck' }),
},
],
};
Expand Down
8 changes: 4 additions & 4 deletions desktop/menus/file-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ const buildFileMenu = (isAuthenticated) => {
label: '&New Note',
visible: isAuthenticated,
accelerator: 'CommandOrControl+Shift+I',
click: appCommandSender({ action: 'newNote' }),
click: appCommandSender('appCommnad', { action: 'newNote' }),
},
{ type: 'separator' },
{
label: '&Import Notes…',
visible: isAuthenticated,
click: appCommandSender({
click: appCommandSender('appCommnad', {
action: 'showDialog',
dialog: 'IMPORT',
}),
Expand All @@ -25,7 +25,7 @@ const buildFileMenu = (isAuthenticated) => {
label: '&Export Notes…',
visible: isAuthenticated,
accelerator: 'CommandOrControl+Shift+E',
click: appCommandSender({
click: appCommandSender('appCommnad', {
action: 'exportNotes',
}),
},
Expand All @@ -34,7 +34,7 @@ const buildFileMenu = (isAuthenticated) => {
label: '&Print…',
visible: isAuthenticated,
accelerator: 'CommandOrControl+P',
click: appCommandSender({ action: 'printNote' }),
click: appCommandSender('appCommnad', { action: 'printNote' }),
},
];

Expand Down
2 changes: 1 addition & 1 deletion desktop/menus/format-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const buildFormatMenu = (isAuthenticated) => {
{
label: 'Insert &Checklist',
accelerator: 'CommandOrControl+Shift+C',
click: appCommandSender({ action: 'insertChecklist' }),
click: appCommandSender('appCommnad', { action: 'insertChecklist' }),
},
];

Expand Down
5 changes: 4 additions & 1 deletion desktop/menus/help-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ const buildHelpMenu = (mainWindow, isAuthenticated) => {
{
label: '&Keyboard Shortcuts',
visible: isAuthenticated,
click: appCommandSender({ action: 'showDialog', dialog: 'KEYBINDINGS' }),
click: appCommandSender('appCommnad', {
action: 'showDialog',
dialog: 'KEYBINDINGS',
}),
},
{ type: 'separator' },
{
Expand Down
8 changes: 4 additions & 4 deletions desktop/menus/menu-items.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const updater = require('../updater');

const about = {
label: '&About ' + app.name,
click: appCommandSender({
click: appCommandSender('appCommnad', {
action: 'showDialog',
dialog: 'ABOUT',
}),
Expand All @@ -20,7 +20,7 @@ const emptyTrash = (isAuthenticated) => {
return {
label: '&Empty Trash',
visible: isAuthenticated,
click: appCommandSender({ action: 'emptyTrash' }),
click: appCommandSender('appCommnad', { action: 'emptyTrash' }),
};
};

Expand All @@ -29,7 +29,7 @@ const preferences = (isAuthenticated) => {
label: 'P&references…',
visible: isAuthenticated,
accelerator: 'CommandOrControl+,',
click: appCommandSender({
click: appCommandSender('appCommnad', {
action: 'showDialog',
dialog: 'SETTINGS',
}),
Expand All @@ -40,7 +40,7 @@ const signout = (isAuthenticated) => {
return {
label: '&Sign Out',
visible: isAuthenticated,
click: appCommandSender({
click: appCommandSender('appCommnad', {
action: 'logout',
}),
};
Expand Down
6 changes: 3 additions & 3 deletions desktop/menus/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const buildRadioGroup = ({ action, propName, settings }) => {
return {
type: 'radio',
checked: id === settings[propName],
click: appCommandSender({
click: appCommandSender('appCommnad', {
action,
[propName]: id,
}),
Expand All @@ -14,10 +14,10 @@ const buildRadioGroup = ({ action, propName, settings }) => {
};
};

const appCommandSender = (arg) => {
const appCommandSender = (commandName = 'appCommand', arg) => {
Copy link
Member

Choose a reason for hiding this comment

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

can we avoid adding optional arguments before required arguments? this can be pretty confusing especially since JS doesn't require a fixed arity when calling functions

return (item, focusedWindow) => {
if (focusedWindow) {
focusedWindow.webContents.send('appCommand', arg);
focusedWindow.webContents.send(commandName, arg);
}
};
};
Expand Down
16 changes: 10 additions & 6 deletions desktop/menus/view-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ const buildViewMenu = (settings, isAuthenticated) => {
label: '&Reversed',
type: 'checkbox',
checked: settings.sortReversed,
click: appCommandSender({ action: 'toggleSortOrder' }),
click: appCommandSender('appCommnad', {
action: 'toggleSortOrder',
}),
},
]),
},
Expand Down Expand Up @@ -96,7 +98,9 @@ const buildViewMenu = (settings, isAuthenticated) => {
label: '&Sort Alphabetically',
type: 'checkbox',
checked: settings.sortTagsAlpha,
click: appCommandSender({ action: 'toggleSortTagsAlpha' }),
click: appCommandSender('appCommnad', {
action: 'toggleSortTagsAlpha',
}),
},
],
},
Expand Down Expand Up @@ -128,19 +132,19 @@ const buildViewMenu = (settings, isAuthenticated) => {
label: 'Zoom &In',
visible: isAuthenticated,
accelerator: 'CommandOrControl+=',
click: appCommandSender({ action: 'increaseFontSize' }),
click: appCommandSender('appCommnad', { action: 'increaseFontSize' }),
},
{
label: 'Zoom &Out',
visible: isAuthenticated,
accelerator: 'CommandOrControl+-',
click: appCommandSender({ action: 'decreaseFontSize' }),
click: appCommandSender('appCommnad', { action: 'decreaseFontSize' }),
},
{
label: '&Actual Size',
visible: isAuthenticated,
accelerator: 'CommandOrControl+0',
click: appCommandSender({ action: 'resetFontSize' }),
click: appCommandSender('appCommnad', { action: 'resetFontSize' }),
},
{
type: 'separator',
Expand All @@ -152,7 +156,7 @@ const buildViewMenu = (settings, isAuthenticated) => {
accelerator: 'CommandOrControl+Shift+F',
type: 'checkbox',
checked: settings.focusModeEnabled,
click: appCommandSender({ action: 'toggleFocusMode' }),
click: appCommandSender('appCommnad', { action: 'toggleFocusMode' }),
},
{
type: 'separator',
Expand Down
1 change: 1 addition & 0 deletions desktop/preload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { contextBridge, ipcRenderer, remote } = require('electron');
const validChannels = [
'appCommand',
'clearCookies',
'editorCommand',
'importNotes',
'noteImportChannel',
'setAutoHideMenuBar',
Expand Down
63 changes: 0 additions & 63 deletions desktop/spellchecker/index.js
View file
Open in desktop

This file was deleted.

15 changes: 15 additions & 0 deletions lib/note-content-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class NoteContentEditor extends Component<Props> {
if (this.bootTimer) {
clearTimeout(this.bootTimer);
}
window.electron?.removeListener('editorCommand');
window.removeEventListener('keydown', this.handleKeys, true);
}

Expand Down Expand Up @@ -189,6 +190,20 @@ class NoteContentEditor extends Component<Props> {
this.editor = editor;
this.monaco = monaco;

window.electron?.receive('editorCommand', (command) => {
switch (command.action) {
case 'redo':
editor.trigger('', 'redo');
return;
case 'selectAll':
editor.setSelection(editor.getModel().getFullModelRange());
return;
case 'undo':
editor.trigger('', 'undo');
return;
}
});

const titleDecoration = (line: number) => ({
range: new monaco.Range(line, 1, line, 1),
options: {
Expand Down