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

Documentation Overhaul #11966

Merged
merged 26 commits into from
Feb 21, 2018
Merged

Documentation Overhaul #11966

merged 26 commits into from
Feb 21, 2018

Conversation

felixrieseberg
Copy link
Member

@felixrieseberg felixrieseberg commented Feb 20, 2018

Apologies for the giant pull request. This is an effort @zeke and I have discussed over and over again and I finally found the time to dig into it.

In essence, this PR takes our existing documentation (everything in the docs/tutorial folder) and creates a clear progression. I have then taken the most obvious holes and filled those in with new documentation. In addition, I have cleaned up all tutorials to use roughly the same style, variable names, and conventions. I also fixed dead links.

This is simply a start. This PR isn't meant to be the end-all of what our documentation looks like, it's merely supposed to be better than what we have today. Today, our documentation is a loose collection of files sorted alphabetically (the "How to build a custom developer tools extension" file comes before the "How to install Electron", because, well, I comes after D...).

📝 Table of contents

In summary:

Also in summary, thanks to @codebytere:

fjpcmty

@felixrieseberg felixrieseberg requested a review from a team February 20, 2018 00:35
a look. It's quite popular in the community and uses `electron-builder`
internally.

## Other Tools and Bboilerplates
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo for boilerplate

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

👍 for the reorganization of the docs


Forge comes with [ready-to-use templates][forge-templates] for popular
frameworks like React, Vue, or Angular. It uses the same core modules used by the
greater Electron community (like [`electron-packager`]) – changes made by
Copy link
Member

Choose a reason for hiding this comment

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

Missing link at the bottom 😄

and renderer processes. Like [`ipcRenderer`](../api/ipc-renderer.md) and
[`ipcMain`](../api/ipc-main.md) modules for sending messages, and the
[remote](../api/remote.md) module for RPC style communication. There is also
an FAQ entry on [how to share data between web pages][share-data].
Copy link
Member

Choose a reason for hiding this comment

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

Missing link

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed!

docs/README.md Outdated
* [Using Node.js APIs](tutorial/application-architecture.md#using-node.js-apis)
* [Using Native Node.js Modules](tutorial/using-native-node-modules.md)
* [Inter-Process Communication](tutorial/application-architecture.md#)
* [Adding Features to Your App]()
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a link?

docs/README.md Outdated
* [Custom Unity Launcher](tutorial/unity-launcher.md)
* [Keyboard Shortcuts](tutorial/keyboard-shortcuts.md)
* [Offline/Online Detection](tutorial/online-offline-events.md)
* [Represented File for macOS Windows](tutorial/represented-file.md)
Copy link
Member

Choose a reason for hiding this comment

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

That is an unfortunately titled document.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be changed. What would be a better title?

Copy link
Member

Choose a reason for hiding this comment

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

Felix changed it to "Represented File for macOS BrowserWindows" here (although I don't think it was changed in the file itself).

@@ -0,0 +1,38 @@
# Unity Launcher Shortcuts
Copy link
Member

Choose a reason for hiding this comment

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

Actions are no longer Unity-specific. They're defined in version 1.1 of the desktop entry spec and they're implemented in the version of GNOME I use (3.22, on Debian).

docs/README.md Outdated

## Tutorials
____
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we prefer --- to ____ in the docs

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote for --- too. We use that on the website to denote the "summary" cut-off point for blog posts. It's a carryover convention from Jekyll.

@zeke
Copy link
Contributor

zeke commented Feb 20, 2018

@felixrieseberg this is so good it almost makes me want to cry. Thank you so much for moving this forward. I'll review it in earnest tomorrow.

## Setting up macOS

> Electron supports Mac OS X 10.9 (and all versions named macOS) and up. Apple does
not allow running macOS in virtual machines unless the host computer is already
Copy link
Contributor

Choose a reason for hiding this comment

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

virtual machines

This blockquote seems like it may not be applicable to the majority of folks who are reading the "Setting up macOS" docs. Seems like more of a concern when it's time to package the app, right?

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've added this because three things kept coming up in my workshops and classes:

  1. People coming in with ancient machines (think Windows XP and 10.8 Macs)
  2. I could usually get those people up and running without replacing their whole machine by giving them a VM
  3. That approach doesn't work on Macs 😢

I'm not married to it though, we can totally take it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough! Leave it in. 👍

require compilation of native code before they can be used) will need to be
compiled to be used with Electron.

The vast majority of Node.js modules is _not_ native. Only 400 out of the
Copy link
Contributor

Choose a reason for hiding this comment

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

are not ?

The vast majority of Node.js modules is _not_ native. Only 400 out of the
~650.000 modules are native. However, if you do need native modules, please
consult [this guide on how to recompile them for Electron][native-node] (it's
easy).
Copy link
Contributor

@YurySolovyov YurySolovyov Feb 20, 2018

Choose a reason for hiding this comment

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

I don't think it's easy thing helps in any way

Copy link
Member

Choose a reason for hiding this comment

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

IMO it makes the docs seem a bit more; friendly 😄

Debugging the main process is a bit trickier, since you cannot simply open
developer tools for them. The Chromium Developer Tools can [be used
to debug Electron's main process][node-inspect] thanks to a closer collaboration
between Google / Chrome and Node.js, but you might encounter oddities like
Copy link
Contributor

Choose a reason for hiding this comment

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

"closer integration between Chrome and Node.js" maybe?

you will merely need Node.js, npm, a code editor of your choice, and a
rudimentary understanding of your operating system's command line client.

## Setting up macOS
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't there be on here? Setting up on macOS, Setting up on Windows, etc.

Copy link
Member Author

@felixrieseberg felixrieseberg Feb 20, 2018

Choose a reason for hiding this comment

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

The point is: "Setting up Windows for Electron development" or "Setting up Windows as a developer environment for Electron."

<h1>Hello World!</h1>
We are using node <script>document.write(process.versions.node)</script>,
Chrome <script>document.write(process.versions.chrome)</script>,
and Electron <script>document.write(process.versions.electron)</script>.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure which API is better, but I think we should not encourage using document.write ever.
Maybe use .innerHTML accessor on some tag?

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 know that it's real tempting to use this PR to change all our docs, but let's not bikeshed this here - this is the existing electron-quick-start and in the current docs. We can totally improve it, but I think this PR is already big enough that I'd like to focus on the changes.

docs/README.md Outdated
* [Mac App Store](tutorial/mac-app-store-submission-guide.md)
* [Windows Store](tutorial/windows-store-guide.md)
* [Snapcraft](tutorial/snapcraft.md)
* [Supported Platforms](tutorial/supported-platforms.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "Supported Platforms" should be the first page in this section?

docs/README.md Outdated

## Detailed Tutorials

These individual tutorial expand on topics discussed in the guide above.
Copy link
Contributor

Choose a reason for hiding this comment

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

plural: tutorials

```javascript
const fs = require('fs')

const root = fs.readdirSync('/')
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this line work on windows? Home about os.homedir() instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does! It returns C:\ by default 👍

docs/README.md Outdated
* [Deploying an Update Server](tutorial/updates.md#deploying-an-update-server)
* [Implementing Updates in Your App](tutorial/updates.md#implementing-updates-in-your-app)
* [Applying Updates](tutorial/updates.md#applying-updates)
* [Application Debugging](tutorial/application-debugging.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you're going off the list I wrote here, but I kinda think the debugging section should come before distribution, security, and updates. And maybe we should give it a name that includes testing, like "Application Testing and Debugging".

On many Linux environments, you can add custom entries to its launcher
by modifying the `.desktop` file. For Canonical's Unity documentation,
see [Adding Shortcuts to a Launcher][unity-launcher]. For details on a
more generic implementation, see the [Free Desktop Specification][spec].
Copy link
Member

Choose a reason for hiding this comment

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

I would say freedesktop.org specification instead of Free Desktop Specification.


## Renderer Process

The most comprehensive tool to debug individual renderer processes are the
Copy link
Contributor

Choose a reason for hiding this comment

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

is / are


That level of modularity and extendability ensures that all developers working
with Electron, both big and small in team-size, are never restricted in what
they can or cannot do at any time during their development life-cycle. However,
Copy link
Contributor

Choose a reason for hiding this comment

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

lifecycle

Copy link
Contributor

@zeke zeke left a comment

Choose a reason for hiding this comment

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

Boy howdy! What a great set of additions.

I found a few typos and made a few suggestions. Feel free to dismiss the big requests though, because as you said this PR is intended to be an improvement on what we already have, rather than the be-all end-all of Electron tutorial documentation.

@@ -0,0 +1,73 @@
# Boilerplates and CLIs

Electron development is un-opinionated - there is no "one true way" to develop,
Copy link
Contributor

Choose a reason for hiding this comment

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

💛


A "complete solution to package and build a ready-for-distribution Electron app"
that focuses on an integrated experience. [`electron-builder`][builder] adds one
single dependency focuses on simplicity and manages all further requirments
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: dependency focuses

## Setting up macOS

> Electron supports Mac OS X 10.9 (and all versions named macOS) and up. Apple does
not allow running macOS in virtual machines unless the host computer is already
Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough! Leave it in. 👍

either the latest `LTS` or `Current` version available. Visit
[the Node.js download page][node-download] and select the `macOS Installer`.
Once downloaded, execute the installer and let the installation wizard guide
you through the installation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a line in here somewhere about how homebrew is not a good avenue for downloading node. I can't remember the exact details but it has something to do with the resultant binary being misnamednodejs, or the install paths being wrong...

as a variant of the Node.js runtime that is focused on desktop applications
instead of web servers.

This doesn't mean Electron is a JavaScript binding to graphical user interface
Copy link
Contributor

Choose a reason for hiding this comment

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

💚

a simple Node application, you would add a `start` script that instructs `node`
to execute the current 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.

I don't feel strongly about it, but this indented JSON style is not consistent what npm does. Any changes like --save would blow this formatting away.

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'm with you. This was in original docs but is easy enough that I fixed it here 👍

const electron = require('electron')
```

The `electron` module exposes is feature in namespaces. The lifecycle of the
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: exposes is feature

@@ -0,0 +1,28 @@
# Represented File for macOS Windows
Copy link
Member

Choose a reason for hiding this comment

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

Could you sync this title with the one in the readme?

@vanessayuenn vanessayuenn merged commit 8e51659 into master Feb 21, 2018
@vanessayuenn vanessayuenn deleted the documentation-overhaul branch February 21, 2018 19:52
@zeke zeke mentioned this pull request Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants