Initial version of Electron versions of basic, app templates #31

clarktlaugh · 2017-04-19T16:56:37Z

Implements two new templates:

electron-basic
electron-app
which are versions of the original 'app' and 'basic' templates that work in Electron.

RyanCCollins · 2017-04-19T17:32:07Z

Wow, nice one!

clarktlaugh · 2017-04-19T17:44:20Z

This is about 99% identical to the 'basic' template. I don't know if there is a straightforward way to accomplish the slight differences without all of the duplication?

clarktlaugh · 2017-04-21T21:46:49Z

I just updated this to include a new 'electron-app' template which is a version of 'app' that works in Electron. It's mostly the same as the original as well.

I've added comments within the review of the code to hopefully call out all of the places that needed to change, since it might be hard to really see them with all of the files showing as new.

clarktlaugh · 2017-04-21T21:48:17Z

templates/electron-basic/webpack.config.babel.js

+  new CopyWebpackPlugin([
+    { from: './public' },
+    { from: './electron-entry.js' },
+    { from: './package.json' },


Lines 10,11 are changed from the original -- to copy the electron entrypoint script and package.json into dist

clarktlaugh · 2017-04-21T21:48:47Z

templates/electron-basic/public/index.html

+  <meta name="mobile-web-app-capable" content="yes">
+  <link rel="shortcut icon" type="image/png" href="/img/shortcut-icon.png" />
+  <link rel="apple-touch-icon" sizes="196x196" type="image/png" href="/img/mobile-app-icon.png">
+  <link rel="stylesheet" type="text/css" href="index.css">


The / needed to be removed

clarktlaugh · 2017-04-21T21:49:06Z

templates/electron-basic/public/index.html

+    }, 100)
+    </script>
+  </div>
+  <script async src="index.js"></script>


The / needed to be removed

clarktlaugh · 2017-04-21T21:49:32Z

templates/electron-basic/package.json

+{
+  "name": "<%= appName %>",
+  "version": "0.1.0",
+  "main": "electron-entry.js",


Changed to the electron entrypoint script

clarktlaugh · 2017-04-21T21:49:47Z

templates/electron-basic/package.json

+    "test": "grommet check",
+    "dev": "cross-env NODE_ENV=development grommet pack",
+    "dist": "cross-env NODE_ENV=production grommet pack",
+    "electron-dev" : "electron ./dist"


A new script to launch the electron app

clarktlaugh · 2017-04-21T21:49:59Z

templates/electron-basic/package.json

+    "sass-lint": "^1.10.2",
+    "sass-loader": "^6.0.3",
+    "webpack": "^2.2.1",
+    "electron": "~1.6.2"


Added electron as a dev dependency

clarktlaugh · 2017-04-21T21:50:43Z

templates/electron-app/webpack.config.babel.js

+
+let plugins = [
+  new CopyWebpackPlugin([{ from: './public' }]),
+  new CopyWebpackPlugin([{ from: './electron-entry.js' }, { from: './package.json' }]),


Line 9 was added to copy the electron entrypoint script and package.json into dist

clarktlaugh · 2017-04-21T21:54:07Z

templates/electron-app/package.json

+    "test": "grommet check",
+    "dev-server": "nodemon ./src/server/dev",
+    "dev": "cross-env NODE_ENV=development grommet pack",
+    "dist": "cross-env NODE_ENV=production grommet pack && npm run dist-server",


Changed dist to include running babel on the server source

clarktlaugh · 2017-04-21T21:54:32Z

templates/electron-app/package.json

+    "sass-lint": "^1.10.2",
+    "sass-loader": "^6.0.3",
+    "webpack": "^2.2.1",
+    "electron": "^1.6.2"


Added electron as a dev dependency

clarktlaugh · 2017-04-21T21:54:43Z

templates/electron-app/public/index.html

+  <meta name="mobile-web-app-capable" content="yes">
+  <link rel="shortcut icon" type="image/png" href="/img/shortcut-icon.png" />
+  <link rel="apple-touch-icon" sizes="196x196" type="image/png" href="/img/mobile-app-icon.png">
+  <link rel="stylesheet" type="text/css" href="index.css">


/ had to be removed from index.css

clarktlaugh · 2017-04-21T21:55:00Z

templates/electron-app/public/index.html

+    }, 100)
+    </script>
+  </div>
+  <script async src="index.js"></script>


/ had to be removed from index.js

clarktlaugh · 2017-04-21T21:56:08Z

templates/electron-app/src/js/api/tasks.js

+if (window.location.protocol === 'https:') {
+  protocol = 'wss:';
+}
+const host = 'localhost:8102'; 


This changed to not try to detect the host the app is running on, because it needs to always talk to localhost since the server is running within Electron/node

clarktlaugh · 2017-04-21T22:03:33Z

templates/electron-app/electron-entry.js

+  console.log(__dirname);
+
+  // and load the index.html of the app.
+  mainWindow.loadURL("http://localhost:8102")


This was changed to point to the server running within the app

is there a way to read the port from a variable?

clarktlaugh · 2017-04-21T22:03:44Z

templates/electron-app/electron-entry.js

+// In this file you can include the rest of your app's specific main process
+// code. You can also put them in separate files and require them here.
+
+require("./server/server");


This was added to start the server

clarktlaugh · 2017-06-14T16:23:41Z

@tracybarmore -- is there anything else I can be doing to help here? There hasn't really been any activity since mid-April.

The changes required to enable this to work for generating an Electron app are extremely minor, but they are in some of the template files, which require completely duplicating all of them (the reason this PR has 72 files). In the comments above, I tried to point out exactly what all of the necessary changes were, and it's only about 15 lines of changes / new code. I do think that having an entirely-new template for this adds a lot of duplication that just isn't necessary, so I could see how that might necessitate a change in the way the CLI handles templates.

One thought I had was to be able to have a common set of template files, then be able to just overwrite a few of them (so 'electron-app' would just use all files from 'app' and then carry around a patch or complete replacement for the changed files. Same for 'electron-basic' and 'basic').

alansouzati · 2017-06-26T20:07:10Z

templates/electron-app/electron-entry.js

@@ -0,0 +1,65 @@
+const electron = require('electron')


in grommet we follow the convention to add a semi-colon at the end of each line. Do you mind following that?

alansouzati · 2017-06-26T20:08:17Z

templates/electron-app/electron-entry.js

+    // Dereference the window object, usually you would store windows
+    // in an array if your app supports multi windows, this is the time
+    // when you should delete the corresponding element.
+    mainWindow = null


in grommet we follow the convention to use undefined instead of null. Do you mind updating that?

alansouzati · 2017-06-26T20:12:57Z

What if we just start with the electron template that is just the basic structure (similar to your electron-basic)? I would stay away from adding a lot of different templates for electron.

I really like the proposal here. But I believe the main goal of this template is to show how to use grommet + electron. There are lot of things in the app template that will deviate the attention more to that than to understand what it takes to run grommet with electron. Hopefully it makes sense.

Thanks for your contribution and sorry for taking such a long time to get back to you on this.

initial version

e12c166

bryanjacquot added the this sprint label Apr 19, 2017

clarktlaugh mentioned this pull request Apr 19, 2017

NWJS grommet/grommet-ferret#41

Open

clarktlaugh changed the title ~~Initial version of electron basic template~~ Initial version of Electron basic template Apr 19, 2017

added an Electron version of the 'app' template

81e3df5

clarktlaugh changed the title ~~Initial version of Electron basic template~~ Initial version of Electron versions of basic, app templates Apr 21, 2017

clarktlaugh commented Apr 21, 2017

View reviewed changes

moved server under src

64f46fc

clarktlaugh commented Apr 21, 2017

View reviewed changes

added missing file

502a95a

clarktlaugh commented Apr 21, 2017

View reviewed changes

tracybarmore added the needs attention label May 1, 2017

tracybarmore removed the needs attention label May 31, 2017

alansouzati reviewed Jun 26, 2017

View reviewed changes

alansouzati removed the this sprint label Jul 5, 2017

L0ZZI added in progress and removed in progress labels Jan 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial version of Electron versions of basic, app templates #31

Initial version of Electron versions of basic, app templates #31

clarktlaugh commented Apr 19, 2017 •

edited by tracybarmore

Loading

RyanCCollins commented Apr 19, 2017

clarktlaugh commented Apr 19, 2017

clarktlaugh commented Apr 21, 2017 •

edited

Loading

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017 •

edited

Loading

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017 •

edited

Loading

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

clarktlaugh Apr 21, 2017

alansouzati Jun 26, 2017

clarktlaugh Apr 21, 2017

clarktlaugh commented Jun 14, 2017

alansouzati Jun 26, 2017

alansouzati Jun 26, 2017

alansouzati commented Jun 26, 2017

Initial version of Electron versions of basic, app templates #31

Are you sure you want to change the base?

Initial version of Electron versions of basic, app templates #31

Conversation

clarktlaugh commented Apr 19, 2017 • edited by tracybarmore Loading

RyanCCollins commented Apr 19, 2017

clarktlaugh commented Apr 19, 2017

clarktlaugh commented Apr 21, 2017 • edited Loading

Choose a reason for hiding this comment

clarktlaugh Apr 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarktlaugh Apr 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarktlaugh commented Jun 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alansouzati commented Jun 26, 2017

clarktlaugh commented Apr 19, 2017 •

edited by tracybarmore

Loading

clarktlaugh commented Apr 21, 2017 •

edited

Loading

clarktlaugh Apr 21, 2017 •

edited

Loading

clarktlaugh Apr 21, 2017 •

edited

Loading