Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

Desktop v3 #100

Merged
merged 31 commits into from
Feb 24, 2016
Merged

Desktop v3 #100

merged 31 commits into from
Feb 24, 2016

Conversation

MadLittleMods
Copy link
Member

@MadLittleMods MadLittleMods commented Feb 5, 2016


Notifications

Windows 10

Windows 8.1

Mac OSX

Ubuntu 14.04.03


Debug nw.js from another PC.

Route localhost:9222 to the external reachable local network. Here are instructions for setting it up on Mac: nwjs/nw.js#1444 (comment)

localhost:9222 -> 192.168.0.58:9222

Then start up the nw.js app:

npm start -- --remote-debugging-port=9222

Visit 192.168.0.58:9222 on the other machine

These parameters also work on a final build.

Windows

"C:\Users\YOUR_USER\AppData\Local\Programs\Gitter\Gitter.exe" --remote-debugging-port=9222 --passthrough-remote-debugging-port=9222

OSX

open /Applications/Gitter.app --args --remote-debugging-port=9222 --passthrough-remote-debugging-port=9222

Windows 10 notifications

Just to dump my knowledge here while it's fresh:

The new notifications in Windows 10 are "toast" notifications. They can be setup by creating a .lnkalso know as "shortcut" in one of the Start menu folders such as "C:\Users\MLM\AppData\Roaming\Microsoft\Windows\Start Menu\Programs".

With nw.js you can use var gui = require('nw.gui'); gui.App.createShortcut(...);(read more here) but we decided to go with node-notifier because it has a sound: false option also know as the "quiet" or "silent" option in their internals. Which can stop the native Windows 10 notification sound from chiming in.

Users can manually manage their notification preferences on a individual basis by opening "Notifications & action settings". http://i.imgur.com/DjNB2oL.png - Because we are using node-notifier the entry is called toast which is the vendor specific exe(find the source here) they use to make the notifications. This does mean that the entry is shared across all apps that use node-notifier but the user shouldn't be touching it in the settings anyway because apps customize their own behaviour (hopefully conscientiously)

@MadLittleMods MadLittleMods changed the title V3 nwjs Desktop V3 Feb 5, 2016
@MadLittleMods MadLittleMods changed the title Desktop V3 Desktop v3 Feb 5, 2016
// And we set to false because Windows 10 by default makes a separate sound
sound: false,
// wait with callback until user action is taken on notification
wait: false
Copy link

Choose a reason for hiding this comment

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

As you are trying to listen for click activation on notification, you'll have to wait for the response from the notification. Without waiting you can't get info on if the user clicks or not. So this should be true.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mikaelbr Thanks for the info. It seems to work just fine with it being wait: false? I believed that option was to make the notification sit there until some user action as taken but that doesn't seem to be the case either.

Copy link
Member Author

Choose a reason for hiding this comment

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

mainWindowFocused = true;
// When a mac app starts up, it doesn't have focus
// When a Windows app starts up, it has focus
// TODO: Check behaviour on linux
Copy link
Contributor

Choose a reason for hiding this comment

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

can you check it on linux please?

Also fixed previous problem when I added `yargs` by installing `normalize-package-data` as a root-level dependency
@MadLittleMods
Copy link
Member Author

@trevorah I updated the auto-update part of the code and have tested the flow from 2.4.0 -> 3.0.0 -> 3.0.1 on OSX. Can you please take a look before I do a Windows release?

I need some sort of code signing identity in my keychain in order to build the OSX app fully but have been just commenting out that part of the code in the mean time.


var checkFileExistSync = function(target) {
try {
return fs.statSync(target);
Copy link
Contributor

Choose a reason for hiding this comment

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

return !!fs.statSync(target); so that we return a boolean

@MadLittleMods
Copy link
Member Author

@trevorah Polling logic review please, latest commit


var MANIFEST_URLS = {
win: 'https://update.gitter.im/win/package.json',
osx: 'https://update.gitter.im/osx/package.json',
linux: 'https://update.gitter.im/linux/package.json'
linux: 'https://update.gitter.im/linux64/package.json'
Copy link
Contributor

Choose a reason for hiding this comment

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

err, what about linux32? whats going on here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We just look at the 64bit manifest for whether to update. The other pieces of the linux build are coupled together so this shouldn't be an issue.

But mainly spawned out of the ./nwapp/utils/client-type.js not distinguishing between the two.

Copy link
Contributor

Choose a reason for hiding this comment

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

thats super confusing. Has this already gone out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly to a few people.

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 updated this to use linux32 and linux64

Also add `osx` package manifest entry (legacy `mac` entry still in place)
MadLittleMods added a commit that referenced this pull request Feb 24, 2016
@MadLittleMods MadLittleMods merged commit d62f774 into master Feb 24, 2016
@MadLittleMods
Copy link
Member Author

MadLittleMods commented Feb 24, 2016

Here was my release process. Merge into master (always do releases from master).

Test each platform release as you go. Also check https://gitter.im/apps to make sure the redirects are in place

OSX

  • gulp build:osx
  • gulp autoupdate:zip:osx
  • gulp redirect:source
  • gulp artefacts:push:osx
  • Don't run gulp redirect:push:osx because we are still using the old safari desktop app
  • gulp autoupdate:push:osx
  • gulp manifest:push:osx

Windows:

  • gulp build:linux
  • gulp cert:fetch:win
  • On a Windows machine: node ./windows/build.js -p thepfxcertpasswordhere (I setup network file sharing from OSX machine (make sure the share name doesn't have a space in it). Use pushd/popd for the UNC path pushd \\ERIC-MACBOOK\eric\Documents\github\desktop)
  • gulp autoupdate:zip:win
  • gulp redirect:source
  • gulp artefacts:push:win
  • gulp redirect:push:win
  • gulp autoupdate:push:win
  • gulp manifest:push:win

Linux

  • gulp build:linux
  • gulp autoupdate:zip:linux
  • gulp redirect:source
  • gulp artefacts:push:linux32
  • gulp artefacts:push:linux64
  • gulp redirect:push:linux32
  • gulp redirect:push:linux64
  • gulp autoupdate:push:linux
  • gulp manifest:push:linux

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants