From 8255ae73b82da871ffe8be926f2bf7c5ebda4be1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felix=20H=C3=A4usler?= <hfelix@protonmail.ch>
Date: Thu, 17 May 2018 04:29:07 +0200
Subject: [PATCH] fix: get menu item by id (#270)

---
 src/main/actions/format.js    | 20 +++++-----
 src/main/actions/paragraph.js | 74 ++++++++++++++++++-----------------
 src/main/actions/view.js      | 36 ++++++++---------
 src/main/menus/edit.js        |  2 +-
 src/main/menus/format.js      |  7 ++++
 src/main/menus/paragraph.js   | 19 +++++++++
 src/main/menus/view.js        |  3 ++
 src/main/utils.js             |  6 +--
 8 files changed, 96 insertions(+), 71 deletions(-)

diff --git a/src/main/actions/format.js b/src/main/actions/format.js
index 3e67888f..09aa65a9 100644
--- a/src/main/actions/format.js
+++ b/src/main/actions/format.js
@@ -1,21 +1,21 @@
 import { ipcMain } from 'electron'
-import { getMenuItem } from '../utils'
+import { getMenuItemById } from '../utils'
 
-const FORMAT_MAP = {
-  'Strong': 'strong',
-  'Emphasis': 'em',
-  'Inline Code': 'inline_code',
-  'Strike': 'del',
-  'Hyperlink': 'link',
-  'Image': 'image'
+const MENU_ID_FORMAT_MAP = {
+  'strongMenuItem': 'strong',
+  'emphasisMenuItem': 'em',
+  'inlineCodeMenuItem': 'inline_code',
+  'strikeMenuItem': 'del',
+  'hyperlinkMenuItem': 'link',
+  'imageMenuItem': 'image'
 }
 
 const selectFormat = formats => {
-  const formatMenuItem = getMenuItem('Format')
+  const formatMenuItem = getMenuItemById('formatMenuItem')
   formatMenuItem.submenu.items.forEach(item => (item.checked = false))
   formatMenuItem.submenu.items
     .forEach(item => {
-      if (formats.some(format => format.type === FORMAT_MAP[item.label])) {
+      if (item.id && formats.some(format => format.type === MENU_ID_FORMAT_MAP[item.id])) {
         item.checked = true
       }
     })
diff --git a/src/main/actions/paragraph.js b/src/main/actions/paragraph.js
index bc9e58b1..4a5bb9d7 100644
--- a/src/main/actions/paragraph.js
+++ b/src/main/actions/paragraph.js
@@ -1,52 +1,54 @@
 import { ipcMain } from 'electron'
-import { getMenuItem } from '../utils'
+import { getMenuItemById } from '../utils'
 
 const DISABLE_LABELS = [
   // paragraph menu items
-  'Heading 1', 'Heading 2', 'Heading 3', 'Heading 4', 'Heading 5', 'Heading 6',
-  'Upgrade Heading Level', 'Degrade Heading Level',
-  'Table',
+  'heading1MenuItem', 'heading2MenuItem', 'heading3MenuItem', 'heading4MenuItem',
+  'heading5MenuItem', 'heading6MenuItem',
+  'upgradeHeadingMenuItem', 'degradeHeadingMenuItem',
+  'tableMenuItem',
   // formats menu items
-  'Hyperlink', 'Image'
+  'hyperlinkMenuItem', 'imageMenuItem'
 ]
 
-const LABEL_MAP = {
-  'Heading 1': 'h1',
-  'Heading 2': 'h2',
-  'Heading 3': 'h3',
-  'Heading 4': 'h4',
-  'Heading 5': 'h5',
-  'Heading 6': 'h6',
-  'Table': 'figure',
-  'Code Fences': 'pre',
-  'Quote Block': 'blockquote',
-  'Order List': 'ol',
-  'Bullet List': 'ul',
-  'Task List': 'ul',
-  'Paragraph': 'p',
-  'Horizontal Line': 'hr',
-  'YAML Front Matter': 'pre'
+const MENU_ID_MAP = {
+  'heading1MenuItem': 'h1',
+  'heading2MenuItem': 'h2',
+  'heading3MenuItem': 'h3',
+  'heading4MenuItem': 'h4',
+  'heading5MenuItem': 'h5',
+  'heading6MenuItem': 'h6',
+  'tableMenuItem': 'figure',
+  'codeFencesMenuItem': 'pre',
+  'quoteBlockMenuItem': 'blockquote',
+  'orderListMenuItem': 'ol',
+  'bulletListMenuItem': 'ul',
+  'taskListMenuItem': 'ul',
+  'paragraphMenuItem': 'p',
+  'horizontalLineMenuItem': 'hr',
+  'frontMatterMenuItem': 'pre'
 }
 
 const setParagraphMenuItemStatus = bool => {
-  const paragraphMenuItem = getMenuItem('Paragraph')
+  const paragraphMenuItem = getMenuItemById('paragraphMenuEntry')
   paragraphMenuItem.submenu.items
     .forEach(item => (item.enabled = bool))
 }
 
-const disableNoMultiple = (disableLabels) => {
-  const paragraphMenuItem = getMenuItem('Paragraph')
-
+const disableNoMultiple = disableLabels => {
+  const paragraphMenuItem = getMenuItemById('paragraphMenuEntry')
   paragraphMenuItem.submenu.items
-    .filter(item => disableLabels.includes(item.label))
+    .filter(item => item.id && disableLabels.includes(item.id))
     .forEach(item => (item.enabled = false))
 }
 
 const setCheckedMenuItem = affiliation => {
-  const paragraphMenuItem = getMenuItem('Paragraph')
+  const paragraphMenuItem = getMenuItemById('paragraphMenuEntry')
   paragraphMenuItem.submenu.items.forEach(item => (item.checked = false))
   paragraphMenuItem.submenu.items.forEach(item => {
-    if (item.label === 'Loose List Item') {
+    if (!item.id) {
+      return false
+    } else if (item.id === 'looseListItemMenuItem') {
       let checked = false
       if (affiliation.length >= 1 && /ul|ol/.test(affiliation[0].type)) {
         checked = affiliation[0].children[0].isLooseListItem
@@ -57,18 +59,18 @@ const setCheckedMenuItem = affiliation => {
     } else if (affiliation.some(b => {
       if (b.type === 'ul') {
         if (b.listType === 'bullet') {
-          return item.label === 'Bullet List'
+          return item.id === 'bulletListMenuItem'
         } else {
-          return item.label === 'Task List'
+          return item.id === 'taskListMenuItem'
         }
       } else if (b.type === 'pre' && b.functionType) {
         if (b.functionType === 'frontmatter') {
-          return item.label === 'YAML Front Matter'
+          return item.id === 'frontMatterMenuItem'
         } else if (b.functionType === 'code') {
-          return item.label === 'Code Fences'
+          return item.id === 'codeFencesMenuItem'
         }
       } else {
-        return b.type === LABEL_MAP[item.label]
+        return b.type === MENU_ID_MAP[item.id]
       }
     })) {
       item.checked = true
@@ -82,7 +84,7 @@ export const paragraph = (win, type) => {
 
 ipcMain.on('AGANI::selection-change', (e, { start, end, affiliation }) => {
   // format menu
-  const formatMenuItem = getMenuItem('Format')
+  const formatMenuItem = getMenuItemById('formatMenuItem')
   formatMenuItem.submenu.items.forEach(item => (item.enabled = true))
   // handle menu checked
   setCheckedMenuItem(affiliation)
@@ -96,10 +98,10 @@ ipcMain.on('AGANI::selection-change', (e, { start, end, affiliation }) => {
     setParagraphMenuItemStatus(false)
   } else if (start.key !== end.key) {
     formatMenuItem.submenu.items
-      .filter(item => DISABLE_LABELS.includes(item.label))
+      .filter(item => item.id && DISABLE_LABELS.includes(item.id))
       .forEach(item => (item.enabled = false))
     disableNoMultiple(DISABLE_LABELS)
   } else if (!affiliation.slice(0, 3).some(p => /ul|ol/.test(p.type))) {
-    disableNoMultiple(['Loose List Item'])
+    disableNoMultiple(['looseListItemMenuItem'])
   }
 })
diff --git a/src/main/actions/view.js b/src/main/actions/view.js
index 5d1c0c80..a3d9a4ba 100644
--- a/src/main/actions/view.js
+++ b/src/main/actions/view.js
@@ -1,23 +1,19 @@
 import { ipcMain, BrowserWindow } from 'electron'
-import { getMenuItem } from '../utils'
+import { getMenuItemById } from '../utils'
 
-const HASH = {
-  'Source Code Mode': 'sourceCode',
-  'Typewriter Mode': 'typewriter',
-  'Focus Mode': 'focus'
-}
+const sourceCodeModeMenuItemId = 'sourceCodeModeMenuItem'
+const typewriterModeMenuItemId = 'typewriterModeMenuItem'
+const focusModeMenuItemId = 'focusModeMenuItem'
 
 export const typeMode = (win, item, type) => {
   const { checked } = item
   win.webContents.send('AGANI::view', { type, checked })
 
   if (type === 'sourceCode') {
-    const viewMenuItem = getMenuItem('View')
-    viewMenuItem.submenu.items.forEach(item => {
-      if (/typewriter|focus/.test(HASH[item.label])) {
-        item.enabled = !checked
-      }
-    })
+    const typewriterModeMenuItem = getMenuItemById(typewriterModeMenuItemId)
+    const focusModeMenuItem = getMenuItemById(focusModeMenuItemId)
+    typewriterModeMenuItem.enabled = !checked
+    focusModeMenuItem.enabled = !checked
   }
 }
 
@@ -26,14 +22,14 @@ export const changeFont = win => {
 }
 
 ipcMain.on('AGANI::ask-for-mode', e => {
-  // format menu
-  const viewMenuItem = getMenuItem('View')
-  const modes = {}
-  viewMenuItem.submenu.items.forEach(item => {
-    if (HASH[item.label]) {
-      modes[HASH[item.label]] = item.checked
-    }
-  })
+  const sourceCodeModeMenuItem = getMenuItemById(sourceCodeModeMenuItemId)
+  const typewriterModeMenuItem = getMenuItemById(typewriterModeMenuItemId)
+  const focusModeMenuItem = getMenuItemById(focusModeMenuItemId)
+  const modes = {
+    sourceCode: sourceCodeModeMenuItem.checked,
+    typewriter: typewriterModeMenuItem.checked,
+    focus: focusModeMenuItem.checked
+  }
   const win = BrowserWindow.fromWebContents(e.sender)
   win.webContents.send('AGANI::res-for-mode', modes)
 })
diff --git a/src/main/menus/edit.js b/src/main/menus/edit.js
index 5a3681ba..a7d8f55e 100755
--- a/src/main/menus/edit.js
+++ b/src/main/menus/edit.js
@@ -50,7 +50,7 @@ export default {
       actions.edit(browserWindow, 'fineNext')
     }
   }, {
-    label: 'FindPrev',
+    label: 'Find Previous',
     accelerator: 'Shift+CmdOrCtrl+U',
     click: (menuItem, browserWindow) => {
       actions.edit(browserWindow, 'findPrev')
diff --git a/src/main/menus/format.js b/src/main/menus/format.js
index 24610d46..a9a43252 100644
--- a/src/main/menus/format.js
+++ b/src/main/menus/format.js
@@ -1,8 +1,10 @@
 import * as actions from '../actions/format'
 
 export default {
+  id: 'formatMenuItem',
   label: 'Format',
   submenu: [{
+    id: 'strongMenuItem',
     label: 'Strong',
     type: 'checkbox',
     accelerator: 'Shift+CmdOrCtrl+B',
@@ -10,6 +12,7 @@ export default {
       actions.format(browserWindow, 'strong')
     }
   }, {
+    id: 'emphasisMenuItem',
     label: 'Emphasis',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+E',
@@ -17,6 +20,7 @@ export default {
       actions.format(browserWindow, 'em')
     }
   }, {
+    id: 'inlineCodeMenuItem',
     label: 'Inline Code',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+`',
@@ -26,6 +30,7 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'strikeMenuItem',
     label: 'Strike',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+D',
@@ -33,6 +38,7 @@ export default {
       actions.format(browserWindow, 'del')
     }
   }, {
+    id: 'hyperlinkMenuItem',
     label: 'Hyperlink',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+L',
@@ -40,6 +46,7 @@ export default {
       actions.format(browserWindow, 'link')
     }
   }, {
+    id: 'imageMenuItem',
     label: 'Image',
     type: 'checkbox',
     accelerator: 'Shift+CmdOrCtrl+I',
diff --git a/src/main/menus/paragraph.js b/src/main/menus/paragraph.js
index ea24ecea..f16c9fc1 100644
--- a/src/main/menus/paragraph.js
+++ b/src/main/menus/paragraph.js
@@ -1,8 +1,10 @@
 import * as actions from '../actions/paragraph'
 
 export default {
+  id: 'paragraphMenuEntry',
   label: 'Paragraph',
   submenu: [{
+    id: 'heading1MenuItem',
     label: 'Heading 1',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+1',
@@ -10,6 +12,7 @@ export default {
       actions.paragraph(browserWindow, 'heading 1')
     }
   }, {
+    id: 'heading2MenuItem',
     label: 'Heading 2',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+2',
@@ -17,6 +20,7 @@ export default {
       actions.paragraph(browserWindow, 'heading 2')
     }
   }, {
+    id: 'heading3MenuItem',
     label: 'Heading 3',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+3',
@@ -24,6 +28,7 @@ export default {
       actions.paragraph(browserWindow, 'heading 3')
     }
   }, {
+    id: 'heading4MenuItem',
     label: 'Heading 4',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+4',
@@ -31,6 +36,7 @@ export default {
       actions.paragraph(browserWindow, 'heading 4')
     }
   }, {
+    id: 'heading5MenuItem',
     label: 'Heading 5',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+5',
@@ -38,6 +44,7 @@ export default {
       actions.paragraph(browserWindow, 'heading 5')
     }
   }, {
+    id: 'heading6MenuItem',
     label: 'Heading 6',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+6',
@@ -47,12 +54,14 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'upgradeHeadingMenuItem',
     label: 'Upgrade Heading',
     accelerator: 'CmdOrCtrl+=',
     click (menuItem, browserWindow) {
       actions.paragraph(browserWindow, 'upgrade heading')
     }
   }, {
+    id: 'degradeHeadingMenuItem',
     label: 'Degrade Heading',
     accelerator: 'CmdOrCtrl+-',
     click (menuItem, browserWindow) {
@@ -61,6 +70,7 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'tableMenuItem',
     label: 'Table',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+T',
@@ -68,6 +78,7 @@ export default {
       actions.paragraph(browserWindow, 'table')
     }
   }, {
+    id: 'codeFencesMenuItem',
     label: 'Code Fences',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+C',
@@ -75,6 +86,7 @@ export default {
       actions.paragraph(browserWindow, 'pre')
     }
   }, {
+    id: 'quoteBlockMenuItem',
     label: 'Quote Block',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+Q',
@@ -84,6 +96,7 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'orderListMenuItem',
     label: 'Order List',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+O',
@@ -91,6 +104,7 @@ export default {
       actions.paragraph(browserWindow, 'ol-order')
     }
   }, {
+    id: 'bulletListMenuItem',
     label: 'Bullet List',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+U',
@@ -98,6 +112,7 @@ export default {
       actions.paragraph(browserWindow, 'ul-bullet')
     }
   }, {
+    id: 'taskListMenuItem',
     label: 'Task List',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+X',
@@ -107,6 +122,7 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'looseListItemMenuItem',
     label: 'Loose List Item',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+L',
@@ -116,6 +132,7 @@ export default {
   }, {
     type: 'separator'
   }, {
+    id: 'paragraphMenuItem',
     label: 'Paragraph',
     type: 'checkbox',
     accelerator: 'CmdOrCtrl+0',
@@ -123,6 +140,7 @@ export default {
       actions.paragraph(browserWindow, 'paragraph')
     }
   }, {
+    id: 'horizontalLineMenuItem',
     label: 'Horizontal Line',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+-',
@@ -130,6 +148,7 @@ export default {
       actions.paragraph(browserWindow, 'hr')
     }
   }, {
+    id: 'frontMatterMenuItem',
     label: 'YAML Front Matter',
     type: 'checkbox',
     accelerator: 'Alt+CmdOrCtrl+Y',
diff --git a/src/main/menus/view.js b/src/main/menus/view.js
index 65226923..a7cbd456 100755
--- a/src/main/menus/view.js
+++ b/src/main/menus/view.js
@@ -28,6 +28,7 @@ let viewMenu = {
   }, {
     type: 'separator'
   }, {
+    id: 'sourceCodeModeMenuItem',
     label: 'Source Code Mode',
     accelerator: 'CmdOrCtrl+U',
     type: 'checkbox',
@@ -36,6 +37,7 @@ let viewMenu = {
       actions.typeMode(browserWindow, item, 'sourceCode')
     }
   }, {
+    id: 'typewriterModeMenuItem',
     label: 'Typewriter Mode',
     accelerator: 'Shift+CmdOrCtrl+T',
     type: 'checkbox',
@@ -44,6 +46,7 @@ let viewMenu = {
       actions.typeMode(browserWindow, item, 'typewriter')
     }
   }, {
+    id: 'focusModeMenuItem',
     label: 'Focus Mode',
     accelerator: 'Shift+CmdOrCtrl+F',
     type: 'checkbox',
diff --git a/src/main/utils.js b/src/main/utils.js
index 6bf63123..e6006aad 100644
--- a/src/main/utils.js
+++ b/src/main/utils.js
@@ -19,11 +19,9 @@ export const getPath = directory => {
   return app.getPath(directory)
 }
 
-export const getMenuItem = menuName => {
-  // TODO(fxha): Please use menu id attribute to find menu entries. This will
-  //             cause problems with internationalization later!
+export const getMenuItemById = menuId => {
   const menus = Menu.getApplicationMenu()
-  return menus.items.find(menu => menu.label === menuName)
+  return menus.getMenuItemById(menuId)
 }
 
 export const log = data => {