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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add login item API #6375

Merged
merged 13 commits into from Jul 8, 2016

Conversation

Projects
None yet
6 participants
@kevinsawicki
Contributor

kevinsawicki commented Jul 6, 2016

Adds 3 2 new APIs to app on macOS 馃崕 that allows apps to add and remove themselves as a login item that will open at login.

Original API

Also adds a getLoginItemStatus API that reports back if the app is currently set as a login item and if it was currently opened automatically at login.

  • app.setAsLoginItem([openAsHidden])
  • app.removeAsLoginItem()
  • app.getLoginItemStatus()

Updated API

  • app.getLoginItemSettings() - returns an object with the app's current settings
  • app.setLoginItemSettings(settings) - sets the app's login item settings, can be used to both add and remove the app as a login item.

/cc @electron/maintainers any naming feedback on these APIs would be appreciated

Closes #6202

@zeke

This comment has been minimized.

Member

zeke commented Jul 6, 2016

What if, instead of setAsLoginItem and removeAsLoginItem, there was a single setLoginItemStatus that expected an object like the one returned by getLoginItemStatus?

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Jul 6, 2016

Can we maybe make it more generic to allow using the same syntax for Windows. It's generally called "Start Up" applications on that side of things so maybe.

app.setAsStartUpItem([openAsHidden])
app.removeAsStartUpItem()
app.isStartUpItem()

Basically just "login items" vs "start up items"

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jul 6, 2016

What if, instead of setAsLoginItem and removeAsLoginItem, there was a single setLoginItemStatus that expected an object like the one returned by getLoginItemStatus?

Yeah, I initially modeled it after the setAsDefaultProtocolClient and removeAsDefaultProtocolClient API names. A single setter could work though and be similar to other APIs that clear state when parameters aren't specified.

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jul 6, 2016

Can we maybe make it more generic to allow using the same syntax for Windows.

Yeah, good point, I'll take a look at adding this on Windows via the registry and think about a common name more.

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jul 6, 2016

Basically just "login items" vs "start up items"

I think login items could potentially work for both Windows and Mac.

From the Windows docs, https://msdn.microsoft.com/en-us/library/windows/desktop/aa376977(v=vs.85).aspx

Run and RunOnce registry keys cause programs to run each time that a user logs on.

And although the user facing verbiage on Windows still mentions startup, login items is a more accurate description of the behavior on both platforms.

@@ -527,8 +527,12 @@ void App::BuildPrototype(
.SetMethod("show", base::Bind(&Browser::Show, browser))
.SetMethod("setUserActivity",
base::Bind(&Browser::SetUserActivity, browser))
.SetMethod("getCurrentActivityType",
base::Bind(&Browser::GetCurrentActivityType, browser))

This comment has been minimized.

@zcbenz

zcbenz Jul 7, 2016

Contributor

Wrong lines removed.

@@ -134,6 +134,15 @@ class Browser : public WindowListObserver {
// Set docks' icon.
void DockSetIcon(const gfx::Image& image);
// Get login item status of app
v8::Local<v8::Value> GetLoginItemStatus(mate::Arguments* args);

This comment has been minimized.

@zcbenz

zcbenz Jul 7, 2016

Contributor

The atom::Browser class should not interactive with V8 directly, APIs added here are also meant to be used by non-V8 parts of code. When type conversions and optional arguments are needed, we usually add a wrapper in api::App.

Set the app as a login item. This will cause the app to be opened automatically
at login.
* `openAsHidden` Boolean - `true` to open the app as hidden. Defaults to

This comment has been minimized.

@zcbenz

zcbenz Jul 7, 2016

Contributor

Parameters should be placed above the description.

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented Jul 7, 2016

Super easy to implement on Linux too, symlink your .desktop file into ~/.config/autostart

dict.Set("openAtLogin", base::mac::CheckLoginItemStatus(&hidden));
dict.Set("openAsHidden", hidden);
dict.Set("restoreState", base::mac::WasLaunchedAsLoginItemRestoreState());
dict.Set("openedAtLogin", base::mac::WasLaunchedAsLoginOrResumeItem());

This comment has been minimized.

@bengotow

bengotow Jul 7, 2016

Contributor

I was a little confused by the different tenses here鈥擨 wonder if calling the keys wasOpenedAtLogin or didOpenAtLogin might make it more clear?

This comment has been minimized.

@kevinsawicki

kevinsawicki Jul 7, 2016

Contributor

I wonder if calling the keys wasOpenedAtLogin or didOpenAtLogin might make it more clear?

馃憤 I really like both suggestions, so how about these:

  • openedAtLogin -> wasOpenedAtLogin
  • openedAsHidden -> wasOpenedAsHidden
@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jul 7, 2016

Thanks everyone for the feedback, the following updates have been made to the APIs.

  • app.getLoginItemStatus() renamed to app.getLoginItemSettings(), same object returned
  • openedAtLogin renamed to wasOpenedAtLogin
  • openedAsHidden renamed to wasOpenedAsHidden
  • app.setAsLoginItem() and app.removeAsLoginItem() have been combined into a app.setLoginItemSettings() API that takes a settings Object with the same keys as app.getLoginItemSettings() and can be used to add/remove the app as a login item.

kevinsawicki added some commits Jul 7, 2016

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Jul 8, 2016

馃憤

@zcbenz zcbenz merged commit 5713e05 into master Jul 8, 2016

8 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
electron-linux-arm Build #3617534 succeeded in 40s
Details
electron-linux-ia32 Build #3617535 succeeded in 37s
Details
electron-linux-x64 Build #3617536 succeeded in 127s
Details
electron-mas-x64 Build #1825 succeeded in 6 min 0 sec
Details
electron-osx-x64 Build #1835 succeeded in 6 min 59 sec
Details
electron-win-ia32 Build #836 succeeded in 6 min 11 sec
Details
electron-win-x64 Build #825 succeeded in 6 min 10 sec
Details

@zcbenz zcbenz deleted the login-item branch Jul 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment