Startup option in preferences (WIP) #837

noamokman · 2016-08-26T05:26:09Z

Hi there,
This is about #835,
Still WIP,
Need to test and create startup on first boot or install.

Would love to help more along the way 😄
Thanks,
Noam

feross · 2016-08-26T05:36:23Z

src/main/startup.js

+function install () {
+  var appPath = process.execPath
+
+  startup.add('webtorrent-desktop', appPath)


Does this work on Windows and OS X?

Yes, Cross-platform (OS X, Linux and Windows)

waiting for typicode/user-startup#7 to land,
apparently all params are required.

noamokman · 2016-08-26T14:42:42Z

@feross where do you think is a good place to have the initial startup installation?
The default value is true so we need to run startup.install at least once.

noamokman · 2016-08-26T15:24:34Z

switched to use auto-launch due to better electron support

noamokman · 2016-08-26T15:37:19Z

@grunjol thanks!
removed the .idea from the gitignore file

feross · 2016-08-26T18:11:00Z

@dcposch do you have thoughts on this?
On Fri, Aug 26, 2016 at 9:48 AM Noam Okman [email protected] wrote:

@feross https://github.com/feross where do you think is a good place to
have the initial startup installation?
The default value is true so we need to run startup.install at least once.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#837 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHbphwBvQkIYGQelERvTUjshk675RK6ks5qjvtigaJpZM4JtwA_
.

grunjol · 2016-08-26T19:45:11Z

src/renderer/lib/state.js

@@ -107,7 +107,8 @@ function setupSavedState (cb) {
      downloadPath: config.DEFAULT_DOWNLOAD_PATH,
      isFileHandler: false,
      openExternalPlayer: false,
-      externalPlayerPath: null
+      externalPlayerPath: null,
+      startup: true


Default should not be true IMHO

Why not? There's a pref to disable

Looks like uTorrent not only starts automatically, but it re-enables that option after every auto-update (!) https://forum.utorrent.com/topic/92281-disable-utorrent-program-auto-start-as-default-setting/

Obv we don't want to do that---if someone turns off the pref, we'll respect it permanently---but I think auto start after they first install WebTorrent is fine.

Let me be clear, I'm not against create a startup feature, but do not force the user beforehand.

The application is not a daemon, so the user should indicate when start.

The user will be surprised with the first autostart (even more if its a updating user).

A lot of users care about their privacy, hence use VPNs, (manually started in most of the cases) so first autostart will be offensive.

Even in the forum post you mention, the users ask for the same expected behavior

he is not complaining about not respecting user's preference but the default behavior.

dcposch · 2016-08-27T01:52:33Z

LGTM! @noamokman thanks for doing this!

For future PRs that add new UI (in this case, new controls on the Prefs page), consider posting a screenshot. Github lets you just paste images directly into a PR, so on Windows you can open the prefs page -> Alt + PrintScr -> paste into Github

noamokman · 2016-08-27T06:14:40Z

@dcposch cool! it looks something like that:

noamokman · 2016-08-27T06:22:42Z

Hi everyone,
That's the remaining work:

test startup on all platforms
- windows
- linux
- mac
disable feature on portable - should I hide hide the pref section ui or show it as disabled?
add migrations function to handle the startup value?
run install on app load (install checks if the auto launch is already enabled first so its safe to run it every single time.

I'm having problems packaging the app on my machine.
So helping at testing the startup on other platforms is appreciated 😄
Still need to decide about the other stuff on the list though.

grunjol · 2016-08-27T12:29:42Z

You don't need to implement the last 2 if the default behavior is not run at startup (simply install or uninstall on preference change)
I can test it in debian and ubuntu.

noamokman · 2016-08-27T13:18:43Z

Yeah, that's true.
Thanks!

noamokman · 2016-08-27T13:53:50Z

@dcposch @grunjol @feross
Final verdict on the default value for startup?
I wanna make adjustments and hopefully merge

dcposch · 2016-08-27T14:19:40Z

Final verdict on the default value for startup?

We can have it off by default, for now, as @grunjol suggested. Sounds like it'll be easier to ship that way. If we make it the default in the future, it will only affect new installs (not existing users). CC @feross

disable feature on portable - should I hide hide the pref section ui or show it as disabled?

I'd just hide it.

test startup on all platforms

I can test on Mac and Windows

grunjol · 2016-08-27T14:35:27Z

src/main/startup.js

+var AutoLaunch = require('auto-launch')
+
+var appLauncher = new AutoLaunch({
+  name: 'WebTorrent Desktop'


you should get the app name from config

grunjol · 2016-08-27T15:19:10Z

auto-start on linux only grabs the first word (WebTorrent) to create the file and launcher... not a big deal.

grunjol · 2016-08-27T15:25:00Z

Working fine on Ubuntu and Debian GNOME.

It starts the app with torrent list. I am guessing... can we make it start 'minimized' from autostart? (maybe with a command line)

noamokman · 2016-08-27T15:37:02Z

Working fine on Ubuntu and Debian GNOME.

Great 👍 thanks

starts the app with torrent list. I am guessing... can we make it start 'minimized' from autostart? (maybe with a command line)

on it!

grunjol · 2016-08-27T15:45:23Z

Reading a little more, it seems it already has an isHidden: true option.
which basically adds a --hidden to the command line.

dcposch · 2016-08-27T15:51:18Z

On Mac (El Capitan) and Windows (Win 10), I'm getting the following error:

module.js:442 Uncaught Error: Cannot find module '../components/Header'

...but only when running the packaged app. npm start works fine.

After testing on the master branch, turns out it's happening there as well.

@feross looks like the React / Material UI change doesn't work with the packaging script yet. I'll try to fix it later today.

grunjol · 2016-08-27T15:52:00Z

already reported #842 its a case sensitive issue

grunjol · 2016-08-27T15:56:01Z

@noamokman processArgv should digest the --hidden parameter and keep it tracked to prevent main.show() being excuted on function onOpen (e, torrentId)

noamokman · 2016-08-27T16:19:19Z

@grunjol oops, didnt see the messages while working on it, check the latest commit.
opening a torrent should show the main window.

dcposch · 2016-08-27T16:19:36Z

After rebasing on top of #843, auto start works on Mac, but it pops up two windows. One is the WebTorrent window (should start minimized), the other is a terminal:

noamokman · 2016-08-27T16:22:38Z

@dcposch weird... maybe merge #843, I'll rebase my branch.
The window should be hidden now. dont know about the terminal though

dcposch · 2016-08-27T16:24:23Z

Works great on Windows!

Looks like there are three things left to do:

disable feature, hide from prefs section in the portable windows app
auto-start minimized
don't start a terminal on Mac

noamokman · 2016-08-27T16:25:11Z

disable feature, hide from prefs section in the portable windows app
auto-start minimized

these two are already pushed

noamokman · 2016-08-27T16:38:06Z

The implementation for linux and windows is an hidden argument.
Not sure what this does for mac:
https://github.com/Teamwork/node-auto-launch/blob/master/src/AutoLaunchMac.coffee#L10

grunjol · 2016-08-27T16:44:44Z

i think you get this wrong, you need to add isHidden: true option in auto-launch and that will add a --hidden parameter on launch

noamokman · 2016-08-27T16:47:29Z

@grunjol yeah I did,
that argument is used to optionally add the --hidden agrument on startup.
not sure it works the same on mac

grunjol · 2016-08-27T16:51:24Z

src/main/index.js

@@ -21,6 +21,7 @@ var windows = require('./windows')

 var shouldQuit = false
 var argv = sliceArgv(process.argv)
+var hidden = argv.includes('--hidden')


instead of set hidden value here, do it in function processArgv (argv), this also prevents --hidden be treated as torrentId

need to leave it here so that the main windows starts hidden,
placing the hide function in the processArgv function makes the window flash for a sec.
Made the processArgv function ignore the value as a torrent id

dcposch · 2016-08-27T17:16:37Z

these two are already pushed

just tried it again--on mac, it still shows the window and the terminal after booting

#843 is merged btw

grunjol · 2016-08-27T17:28:24Z

@dcposch terminal showing issue:

Teamwork/node-auto-launch#28

Check the latest message it has a possible solution

noamokman · 2016-08-28T02:40:50Z

waiting for Teamwork/node-auto-launch#38 to land 👍
also rebased the branch.
sorted the process args function ot ignore --hidden

dcposch · 2016-09-14T02:54:12Z

Rebased, added some fixes, merged. See #923 . Thanks @noamokman !

noamokman mentioned this pull request Aug 26, 2016

Start the app automatically on boot #835

Closed

feross reviewed Aug 26, 2016
View reviewed changes

grunjol reviewed Aug 26, 2016
View reviewed changes

grunjol reviewed Aug 27, 2016
View reviewed changes

jurgentreep mentioned this pull request Aug 31, 2016

npm run watch failing to execute #861

Closed

added a startup option

02a2473

noamokman mentioned this pull request Sep 2, 2016

make "npm run watch" work on windows #878

Merged

dcposch mentioned this pull request Sep 14, 2016

Start automatically on login #923

Merged

dcposch closed this Sep 14, 2016

noamokman deleted the startup branch September 24, 2016 05:05

lock bot locked as resolved and limited conversation to collaborators May 20, 2018

Startup option in preferences (WIP) #837

Startup option in preferences (WIP) #837

Conversation

noamokman commented Aug 26, 2016

feross Aug 26, 2016

Choose a reason for hiding this comment

noamokman Aug 26, 2016 • edited Loading

Choose a reason for hiding this comment

noamokman Aug 26, 2016

Choose a reason for hiding this comment

noamokman commented Aug 26, 2016

noamokman commented Aug 26, 2016

noamokman commented Aug 26, 2016

feross commented Aug 26, 2016

grunjol Aug 26, 2016

Choose a reason for hiding this comment

dcposch Aug 27, 2016

Choose a reason for hiding this comment

grunjol Aug 27, 2016 • edited Loading

Choose a reason for hiding this comment

dcposch commented Aug 27, 2016

noamokman commented Aug 27, 2016

noamokman commented Aug 27, 2016

grunjol commented Aug 27, 2016 • edited Loading

noamokman commented Aug 27, 2016

noamokman commented Aug 27, 2016

dcposch commented Aug 27, 2016

grunjol Aug 27, 2016

Choose a reason for hiding this comment

grunjol commented Aug 27, 2016 • edited Loading

grunjol commented Aug 27, 2016

noamokman commented Aug 27, 2016

grunjol commented Aug 27, 2016

dcposch commented Aug 27, 2016 • edited Loading

grunjol commented Aug 27, 2016 • edited Loading

grunjol commented Aug 27, 2016 • edited Loading

noamokman commented Aug 27, 2016

dcposch commented Aug 27, 2016

noamokman commented Aug 27, 2016

dcposch commented Aug 27, 2016

noamokman commented Aug 27, 2016 • edited Loading

noamokman commented Aug 27, 2016

grunjol commented Aug 27, 2016 • edited Loading

noamokman commented Aug 27, 2016 • edited Loading

grunjol Aug 27, 2016 • edited Loading

Choose a reason for hiding this comment

noamokman Aug 28, 2016

Choose a reason for hiding this comment

dcposch commented Aug 27, 2016 • edited Loading

grunjol commented Aug 27, 2016 • edited Loading

noamokman commented Aug 28, 2016 • edited Loading

dcposch commented Sep 14, 2016

noamokman Aug 26, 2016 •

edited

Loading

grunjol Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

dcposch commented Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

noamokman commented Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

noamokman commented Aug 27, 2016 •

edited

Loading

grunjol Aug 27, 2016 •

edited

Loading

dcposch commented Aug 27, 2016 •

edited

Loading

grunjol commented Aug 27, 2016 •

edited

Loading

noamokman commented Aug 28, 2016 •

edited

Loading