Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 20 additions & 33 deletions src/auto-updater/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { app, ipcMain, Menu } = require('electron')
const { app, ipcMain } = require('electron')
const { rootPath: appDir } = require('electron-root-path')
const fs = require('fs')
const path = require('path')
Expand Down Expand Up @@ -40,6 +40,7 @@ const ThemeIpcChannelHandlers = require(
)

const MENU_ITEM_IDS = require('../create-menu/menu.item.ids')
const { changeMenuItemStatesById } = require('../create-menu/utils')

const isAutoUpdateDisabled = parseEnvValToBool(process.env.IS_AUTO_UPDATE_DISABLED)

Expand Down Expand Up @@ -231,46 +232,32 @@ const _fireToast = (
return res
}

const _getUpdateMenuItemById = (id) => {
const menu = Menu.getApplicationMenu()

if (
!menu ||
!id ||
typeof id !== 'string'
) {
return
}

return menu.getMenuItemById(id)
}

const _switchMenuItem = (opts = {}) => {
const _switchMenuItem = (opts) => {
const {
isCheckMenuItemDisabled,
isInstallMenuItemVisible
} = { ...opts }
const checkMenuItem = _getUpdateMenuItemById(
MENU_ITEM_IDS.CHECK_UPDATE_MENU_ITEM
)
const installMenuItem = _getUpdateMenuItemById(
MENU_ITEM_IDS.INSTALL_UPDATE_MENU_ITEM
)

if (
!checkMenuItem ||
!installMenuItem
) {
return
}
} = opts ?? {}
const checkMenuItemOpts = {}
const installMenuItemOpts = {}

if (typeof isCheckMenuItemDisabled === 'boolean') {
checkMenuItem.enabled = !isCheckMenuItemDisabled
checkMenuItemOpts.enabled = !isCheckMenuItemDisabled
}
if (typeof isInstallMenuItemVisible === 'boolean') {
checkMenuItem.visible = !isInstallMenuItemVisible
installMenuItem.visible = isInstallMenuItemVisible
checkMenuItemOpts.visible = !isInstallMenuItemVisible
installMenuItemOpts.visible = isInstallMenuItemVisible
}

changeMenuItemStatesById([
{
menuItemId: MENU_ITEM_IDS.CHECK_UPDATE_MENU_ITEM,
opts: checkMenuItemOpts
},
{
menuItemId: MENU_ITEM_IDS.INSTALL_UPDATE_MENU_ITEM,
opts: installMenuItemOpts
}
])
}

const _reinitInterval = () => {
Expand Down
10 changes: 5 additions & 5 deletions src/changelog-manager/show-changelog.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const { Menu } = require('electron')
const path = require('path')
const parseChangelog = require('changelog-parser')
const { rootPath } = require('electron-root-path')
Expand All @@ -9,14 +8,15 @@ const getDebugInfo = require('../helpers/get-debug-info')
const showDocs = require('../show-docs')

const MENU_ITEM_IDS = require('../create-menu/menu.item.ids')
const { changeMenuItemStatesById } = require('../create-menu/utils')

const changelogPath = path.join(rootPath, 'CHANGELOG.md')

const disableShowChangelogMenuItem = () => {
const menuItem = Menu.getApplicationMenu()
?.getMenuItemById(MENU_ITEM_IDS.SHOW_CHANGE_LOG_MENU_ITEM) ?? {}

menuItem.enabled = false
changeMenuItemStatesById(
MENU_ITEM_IDS.SHOW_CHANGE_LOG_MENU_ITEM,
{ enabled: false }
)
}

module.exports = async (params = {}) => {
Expand Down
53 changes: 53 additions & 0 deletions src/create-menu/utils/change-menu-item-states-by-id.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict'

const { Menu, MenuItem } = require('electron')

const MenuIpcChannelHandlers = require(
'../../window-creators/main-renderer-ipc-bridge/menu-ipc-channel-handlers'
)
const wins = require('../../window-creators/windows')
const WINDOW_NAMES = require('../../window-creators/window.names')
const isMainWinAvailable = require(
'../../helpers/is-main-win-available'
)

module.exports = (menuItemId, opts) => {

Choose a reason for hiding this comment

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

I would suggest always accepting an array ([{ menuItemId, opts }, ...], fallbackOpts) in this method since we create an array from a single item anyway. IMHO, parameters list is misleading otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not against strict typing, but I'd like to keep some flexibility here as it is
Since this is pure JS, we should rely on the Defensive Programming, and this is an old and reliable way of guaranteeing an array in this place

Choose a reason for hiding this comment

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

I am up for different styles and paradigms. I'm this particular case I'm missing the purpose of m multiple types accepted since it always creates an array anyway. At the same time, there is only one place where method is invoked with an array and it could be easily modified to have multiple calls instead. Additionally, opts logic with the current approach is not straightforward and serves multiple purposes simultaneously, which is not ideal.
After all, to me the current implementation looks unnecessarily overcomplicated, which translates to maintenance problems eventually

Copy link
Member Author

Choose a reason for hiding this comment

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

modified to have multiple calls instead not quite so as at the end, we should emit one rerender menu event

const menu = Menu.getApplicationMenu()
const args = Array.isArray(menuItemId)
? menuItemId
: [{ menuItemId, opts }]

if (
!(menu instanceof Menu) ||
args.length === 0
) {
return
}

for (const item of args) {
const id = item?.menuItemId
const props = {
...item?.opts,
...opts
}

if (!id) {
continue
}

const menuItem = id instanceof MenuItem
? id
: menu.getMenuItemById(id)

for (const [name, val] of Object.entries(props)) {
menuItem[name] = val
}
}

if (!isMainWinAvailable()) {
return
}

MenuIpcChannelHandlers
.sendRerenderMenuEvent(wins[WINDOW_NAMES.MAIN_WINDOW])
}
9 changes: 9 additions & 0 deletions src/create-menu/utils/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict'

const changeMenuItemStatesById = require(
'./change-menu-item-states-by-id'
)

module.exports = {
changeMenuItemStatesById
}
25 changes: 12 additions & 13 deletions src/error-manager/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { app, Menu } = require('electron')
const { app } = require('electron')
const os = require('os')
const cleanStack = require('clean-stack')
const i18next = require('i18next')
Expand All @@ -20,6 +20,7 @@ const collectLogs = require('./collect-logs')
const getDebugInfo = require('../helpers/get-debug-info')

const MENU_ITEM_IDS = require('../create-menu/menu.item.ids')
const { changeMenuItemStatesById } = require('../create-menu/utils')

let _isLocked = false
let _isIssueAutoManagerLocked = false
Expand Down Expand Up @@ -97,24 +98,22 @@ const _isLogSkipped = (log) => {
)
}

const _getReportBugMenuItem = () => {
const menu = Menu.getApplicationMenu()

if (!menu) {
return {}
}

return menu.getMenuItemById(MENU_ITEM_IDS.REPORT_BUG_MENU_ITEM) || {}
}

const _lockIssueManager = () => {
_isLocked = true
_getReportBugMenuItem().enabled = false

changeMenuItemStatesById(
MENU_ITEM_IDS.REPORT_BUG_MENU_ITEM,
{ enabled: false }
)
}

const _unlockIssueManager = () => {
_isLocked = false
_getReportBugMenuItem().enabled = true

changeMenuItemStatesById(
MENU_ITEM_IDS.REPORT_BUG_MENU_ITEM,
{ enabled: true }
)
}

const manageNewGithubIssue = async (params) => {
Expand Down
3 changes: 2 additions & 1 deletion src/helpers/is-main-win-available.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict'

const wins = require('../window-creators/windows')
const WINDOW_NAMES = require('../window-creators/window.names')

module.exports = (
win = wins?.mainWindow,
win = wins?.[WINDOW_NAMES.MAIN_WINDOW],
opts = {}
) => {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ class MenuIpcChannelHandlers extends IpcChannelHandlers {
static sendHideMenuEvent (win, args) {
return this.sendToRenderer(this.sendHideMenuEvent, win, args)
}

static sendRerenderMenuEvent (win, args) {
return this.sendToRenderer(this.sendRerenderMenuEvent, win, args)
}
}

module.exports = MenuIpcChannelHandlers
3 changes: 2 additions & 1 deletion src/window-creators/main-renderer-ipc-bridge/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const MENU_INVOKE_METHOD_NAMES = {
EXEC_MENU_CMD: 'execMenuCmd'
}
const MENU_EVENT_METHOD_NAMES = {
ON_HIDE_MENU_EVENT: 'onHideMenuEvent'
ON_HIDE_MENU_EVENT: 'onHideMenuEvent',
ON_RERENDER_MENU_EVENT: 'onRerenderMenuEvent'
}
const THEME_INVOKE_METHOD_NAMES = {
SET_THEME: 'setTheme',
Expand Down