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.

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

feat(ux): move preferences to menubar #1425

Merged
merged 4 commits into from
Apr 24, 2020
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
54 changes: 53 additions & 1 deletion assets/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"readReleaseNotes": "Read Release Notes",
"status": "Status",
"files": "Files",
"settings": "Settings",
Copy link
Member

@lidel lidel Apr 21, 2020

Choose a reason for hiding this comment

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

Removing this from the top level menu may be a problem. I believe users will look for the "WebUI Settings" screen under "Settings" and won't find any entry point there.

Can we add it at the very top of new "Settings" submenu, to maintain continuity, of sorts?

One entry followed by a separator and then existing entries under "Desktop" header (similar to existing "Experiments"):

2020-04-21--03-12-26

@jessicaschilling Does this make sense? If so, what would be the correct wording here: "Open Node Settings"? "Open WebUI Settings"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just rebased on top of master and added a "Open Node Settings" which opens Web UI settings:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

This is great. Agree on “Node Settings” over “WebUI settings sine the thing you’re adjusting is the node itself.

Copy link
Member

@lidel lidel Apr 21, 2020

Choose a reason for hiding this comment

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

How about adding Desktop Integrations header below the separator,
to make the grouping even more clear?

png

Copy link
Member Author

Choose a reason for hiding this comment

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

Just added:
image

Just wondering if it's really needed. But looks great

Copy link
Contributor

Choose a reason for hiding this comment

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

Having a clarification of “these are the prefs for this app” is a good idea. I might just say “App Preferences” though? Not sure if “desktop integrations” makes sense unless you’re already deeply familiar.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lidel what do you think?

"peers": "Peers",
"quit": "Quit",
"versions": "Versions",
"screenshotTaken": "Screenshot taken",
Expand All @@ -32,6 +32,7 @@
"close": "Close",
"ok": "OK",
"cancel": "Cancel",
"enable": "Enable",
"reportTheError": "Report the error",
"restartIpfsDesktop": "Restart IPFS Desktop",
"openLogs": "Open logs",
Expand Down Expand Up @@ -171,5 +172,56 @@
"couldNotSaveDialog": {
"title": "Could not write to disk",
"message": "There was an error writing to the disk. Please try again."
},
"launchAtLoginNotSupported": {
"title": "Error",
"message": "Launch at login is not supported on your platform."
},
"launchAtLoginFailed": {
"title": "Error",
"message": "Launch at login could not be enabled on your machine."
},
"enableIpfsOnPath": {
"title": "Enable IPFS on PATH",
"message": "By enabling this option, IPFS will be available on your command line as \"ipfs\". This action is reversible.",
"action": "Enable"
},
"disableIpfsOnPath": {
"title": "Disable IPFS on PATH",
"message": "By disabling this option, IPFS will no longer be available on your command line as \"ipfs\".",
"action": "Disable"
},
"enableGlobalTakeScreenshotShortcut": {
"title": "Enable screenshot shortcut",
"message": "By enabling this, the shortcut \"{ accelerator }\" will be available to take screenshots as long as IPFS Desktop is running."
},
"enableGlobalDownloadShortcut": {
"title": "Enable download shortcut",
"message": "By enabling this, the shortcut \"{ accelerator }\" will be available to download files as long as IPFS Desktop is running."
},
"installNpmOnIpfsWarning": {
"title": "Install npm on IPFS",
"message": "This experimental feature installs the \"ipfs-npm\" package on your system. It requires Node.js to be installed.",
"action": "Install"
},
"unableToInstallNpmOnIpfs": {
"title": "Error",
"message": "It was not possible to install \"ipfs-npm\" package on your system. Please check the logs for more information or try installing it manually by running \"npm install -g ipfs-npm\" on your command line."
},
"unableToUninstallNpmOnIpfs": {
"title": "Error",
"message": "It was not possible to uninstall \"ipfs-npm\" package on your system. Please check the logs for more information or try uninstalling it manually by running \"npm uninstall -g ipfs-npm\" on your command line."
},
"settings": {
"settings": "Settings",
"preferences": "Preferences",
"openNodeSettings": "Open Node Settings",
"appPreferences": "App Preferences",
"launchOnStartup": "Launch at Login",
"ipfsCommandLineTools": "Command Line Tools",
"takeScreenshotShortcut": "Global Screenshot Shortcut",
"downloadHashShortcut": "Global Download Shortcut",
"experiments": "Experiments",
"npmOnIpfs": "npm on IPFS"
}
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"clean:webui": "shx rm -rf assets/webui/",
"build": "run-s clean:webui build:*",
"build:webui": "run-s build:webui:*",
"build:webui:download": "npx ipfs-or-gateway -c bafybeidatpz2hli6fgu3zul5woi27ujesdf5o5a7bu622qj6ugharciwjq -p assets/webui/ -t 360000 --verbose",
"build:webui:download": "npx ipfs-or-gateway -c bafybeicevz7mmgt45zitnmg73khkmvioz2ozixvd6guujphhtsenu2pt2a -p assets/webui/ -t 360000 --verbose",
"build:webui:minimize": "shx rm -rf assets/webui/static/js/*.map && shx rm -rf assets/webui/static/css/*.map",
"build:binaries": "electron-builder --publish onTag"
},
Expand Down
43 changes: 37 additions & 6 deletions src/auto-launch.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { app } = require('electron')
const i18n = require('i18next')
const os = require('os')
const path = require('path')
const fs = require('fs-extra')
Expand All @@ -7,6 +8,7 @@ const createToggler = require('./utils/create-toggler')
const logger = require('./common/logger')
const store = require('./common/store')
const { IS_MAC, IS_WIN } = require('./common/consts')
const { showDialog, recoverableErrorDialog } = require('./dialogs')

const CONFIG_KEY = 'autoLaunch'

Expand Down Expand Up @@ -47,22 +49,40 @@ async function disable () {
await fs.remove(getDesktopFile())
}

module.exports = async function (ctx) {
const activate = async (value, oldValue) => {
module.exports = async function () {
const activate = async ({ newValue, oldValue, feedback }) => {
if (process.env.NODE_ENV === 'development') {
logger.info('[launch on startup] unavailable during development')

if (feedback) {
showDialog({
title: 'Launch at Login',
message: 'Not available during development.',
buttons: [i18n.t('close')]
})
}

return
}

if (!isSupported()) {
logger.info('[launch on startup] not supported on this platform')

if (feedback) {
showDialog({
title: i18n.t('launchAtLoginNotSupported.title'),
message: i18n.t('launchAtLoginNotSupported.message'),
buttons: [i18n.t('close')]
})
}

return false
}

if (value === oldValue) return
if (newValue === oldValue) return

try {
if (value === true) {
if (newValue === true) {
await enable()
logger.info('[launch on startup] enabled')
} else {
Expand All @@ -73,10 +93,21 @@ module.exports = async function (ctx) {
return true
} catch (err) {
logger.error(`[launch on startup] ${err.toString()}`)

if (feedback) {
recoverableErrorDialog(err, {
title: i18n.t('launchAtLoginFailed.title'),
message: i18n.t('launchAtLoginFailed.message')
})
}

return false
}
}

activate(store.get(CONFIG_KEY, false))
createToggler(ctx, CONFIG_KEY, activate)
activate({ newValue: store.get(CONFIG_KEY, false) })
createToggler(CONFIG_KEY, activate)
}

module.exports.CONFIG_KEY = CONFIG_KEY
module.exports.isSupported = isSupported
14 changes: 8 additions & 6 deletions src/dialogs/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function criticalErrorDialog (e) {
// Shows a recoverable error dialog with the default title and message.
// Passing an options object alongside the error can be used to override
// the title and message.
function recoverableErrorDialog (e, options = {}) {
function recoverableErrorDialog (e, options) {
const cfg = {
title: i18n.t('recoverableErrorDialog.title'),
message: i18n.t('recoverableErrorDialog.message'),
Expand All @@ -59,12 +59,14 @@ function recoverableErrorDialog (e, options = {}) {
]
}

if (options.title) {
cfg.title = options.title
}
if (options) {
if (options.title) {
cfg.title = options.title
}

if (options.message) {
cfg.message = options.message
if (options.message) {
cfg.message = options.message
}
}

const option = dialog(cfg)
Expand Down
7 changes: 6 additions & 1 deletion src/download-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ async function downloadCid (ctx) {
}

module.exports = function (ctx) {
setupGlobalShortcut(ctx, {
setupGlobalShortcut({
confirmationDialog: {
title: i18n.t('enableGlobalDownloadShortcut.title'),
message: i18n.t('enableGlobalDownloadShortcut.message', { accelerator: SHORTCUT })
},
settingsOption: CONFIG_KEY,
accelerator: SHORTCUT,
action: () => {
Expand All @@ -151,3 +155,4 @@ module.exports = function (ctx) {

module.exports.downloadCid = downloadCid
module.exports.SHORTCUT = SHORTCUT
module.exports.CONFIG_KEY = CONFIG_KEY
55 changes: 45 additions & 10 deletions src/ipfs-on-path/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,57 @@ const execOrSudo = require('../utils/exec-or-sudo')
const logger = require('../common/logger')
const store = require('../common/store')
const { IS_WIN } = require('../common/consts')
const { recoverableErrorDialog } = require('../dialogs')
const { showDialog, recoverableErrorDialog } = require('../dialogs')

const CONFIG_KEY = 'ipfsOnPath'

module.exports = async function (ctx) {
createToggler(ctx, CONFIG_KEY, async (value, oldValue) => {
if (value === oldValue || (oldValue === null && !value)) return
if (value === true) return run('install')
const errorMessage = {
title: i18n.t('cantAddIpfsToPath.title'),
message: i18n.t('cantAddIpfsToPath.message')
}

module.exports = async function () {
createToggler(CONFIG_KEY, async ({ newValue, oldValue }) => {
if (newValue === oldValue || (oldValue === null && !newValue)) {
return
}

if (newValue === true) {
if (showDialog({
title: i18n.t('enableIpfsOnPath.title'),
message: i18n.t('enableIpfsOnPath.message'),
buttons: [
i18n.t('enableIpfsOnPath.action'),
i18n.t('cancel')
]
}) !== 0) {
// User canceled
return
}

return run('install')
}

if (showDialog({
title: i18n.t('disableIpfsOnPath.title'),
message: i18n.t('disableIpfsOnPath.message'),
buttons: [
i18n.t('disableIpfsOnPath.action'),
i18n.t('cancel')
]
}) !== 0) {
// User canceled
return
}

return run('uninstall')
})

firstTime()
}

module.exports.CONFIG_KEY = CONFIG_KEY

async function firstTime () {
// Check if we've done this before.
if (store.get(CONFIG_KEY, null) !== null) {
Expand Down Expand Up @@ -54,10 +91,7 @@ async function runWindows (script, { failSilently }) {
logger.error(`[ipfs on path] ${err.toString()}`)

if (!failSilently) {
recoverableErrorDialog(err, {
title: i18n.t('cantAddIpfsToPath.title'),
message: i18n.t('cantAddIpfsToPath.message')
})
recoverableErrorDialog(err, errorMessage)
}

return resolve(false)
Expand All @@ -78,6 +112,7 @@ async function run (script, { trySudo = true, failSilently = false } = {}) {
script: join(__dirname, `./scripts/${script}.js`),
scope: 'ipfs on path',
trySudo,
failSilently
failSilently,
errorOptions: errorMessage
})
}
73 changes: 44 additions & 29 deletions src/npm-on-ipfs/index.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,32 @@
const which = require('which')
const i18n = require('i18next')
const pkg = require('./package')
const logger = require('../common/logger')
const store = require('../common/store')
const { showDialog } = require('../dialogs')
const createToggler = require('../utils/create-toggler')

const CONFIG_KEY = 'experiments.npmOnIpfs'

module.exports = function (ctx) {
let interval = null
// Every 12 hours, check if `ipfs-npm` is installed and, if it is,
// tries to update it to the latest version.
setInterval(existsAndUpdate, 43200000)

createToggler(ctx, CONFIG_KEY, async (value, oldValue) => {
if (value === oldValue || oldValue === null) return true
// Configure toggler
createToggler(CONFIG_KEY, toggle)

// If the user is telling to (un)install even though they have (un)installed
// ipfs-npm package manually.
const manual = isPkgInstalled() === value

if (value === true) {
if (!manual && !await pkg.install()) return false
interval = setInterval(existsAndUpdate, 43200000) // every 12 hours
return true
}

clearInterval(interval)
return manual || pkg.uninstall()
})

let opt = store.get(CONFIG_KEY, null)
const exists = isPkgInstalled()

if (opt === null) {
// When running for the first time, update the config to know if `ipfs-npm`
// is installed or not.
if (store.get(CONFIG_KEY, null) === null) {
const exists = isPkgInstalled()
logger.info(`[npm on ipfs] 1st time running and package is ${exists ? 'installed' : 'not installed'}`)
store.set(CONFIG_KEY, exists)
opt = exists
}

if (opt === true) {
logger.info('[npm on ipfs] set to update every 12 hours')
interval = setInterval(existsAndUpdate, 43200000) // every 12 hours
} else {
logger.info('[npm on ipfs] no action taken')
}
}

module.exports.CONFIG_KEY = CONFIG_KEY

function isPkgInstalled () {
return !!which.sync('ipfs-npm', { nothrow: true })
}
Expand All @@ -54,3 +38,34 @@ function existsAndUpdate () {
store.set(CONFIG_KEY, false)
}
}

async function toggle ({ newValue, oldValue }) {
if (newValue === oldValue || oldValue === null) {
return true
}

// If the user is telling to (un)install even though they have (un)installed
// ipfs-npm package manually.
const manual = isPkgInstalled() === newValue

if (!newValue) {
return manual || pkg.uninstall()
}

const opt = showDialog({
type: 'warning',
title: i18n.t('installNpmOnIpfsWarning.title'),
message: i18n.t('installNpmOnIpfsWarning.message'),
buttons: [
i18n.t('installNpmOnIpfsWarning.action'),
i18n.t('cancel')
]
})

if (opt !== 0) {
// User canceled
return
}

return manual || pkg.install()
}
Loading