Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Initial Electron Settings - for Auto Launch #920

Merged
merged 6 commits into from
May 30, 2017
Merged

Initial Electron Settings - for Auto Launch #920

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1186207
Initial Electron Settings - for Auto Launch
t3chguy May 24, 2017
b9ac122
remove listener on unmount
t3chguy May 24, 2017
a2229ce
Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…
t3chguy May 29, 2017
7e02977
i18n
t3chguy May 29, 2017
0d930c0
change wording and i18n it
t3chguy May 30, 2017
dcd99ac
Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…
t3chguy May 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/BasePlatform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default class BasePlatform {
* Returns a promise that resolves to a string representing
* the current version of the application.
*/
getAppVersion() {
getAppVersion(): Promise<string> {
throw new Error("getAppVersion not implemented!");
}

Expand All @@ -79,10 +79,12 @@ export default class BasePlatform {
* with getUserMedia, return a string explaining why not.
* Otherwise, return null.
*/
screenCaptureErrorString() {
screenCaptureErrorString(): string {
return "Not implemented";
}

isElectron(): boolean { return false; }
Copy link
Member

Choose a reason for hiding this comment

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

this feels a bit weird to pollute BasePlatform with knowledge of electron - could we not just check the type of PlatformPeg to see if it's a VectorBasePlatform or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

how do you mean? I can't do PlatformPeg.get() instanceof ElectronPlatform from the react-sdk side of things. How about a generic getFlags() method on BasePlatform and descendants that would have a prop of isElectron = true on Electron Platform, no idea what else it'd be useful for but it seems like the most generic way of solving this

Copy link
Member Author

Choose a reason for hiding this comment

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

or since now they have getHumanReadableName could do it based off of that but that feels dirty

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 could also just be a property and then accessed as PlatformPeg.get().isElectron
that way no other classes have to implement it but is also weird, I guess its all weird with the current settings architecture

Copy link
Member

Choose a reason for hiding this comment

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

i must be missing how instanceof works - why wouldn't it work from the react-sdk side?

Copy link
Member Author

Choose a reason for hiding this comment

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

how would I import ElectronPlatform from riot-web into react-sdk so I can refer to it in an instanceof? Am I missing something obvious :/?

Copy link
Member

Choose a reason for hiding this comment

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

only by violating the layering (which already happens in a bunch of places, sadly). for now, stick with the isElectron() method.


/**
* Restarts the application, without neccessarily reloading
* any application code
Expand Down
42 changes: 42 additions & 0 deletions src/components/structures/UserSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ module.exports = React.createClass({
this._syncedSettings = syncedSettings;

this._localSettings = UserSettingsStore.getLocalSettings();

if (PlatformPeg.get().isElectron()) {
const {ipcRenderer} = require('electron');

ipcRenderer.on('settings', this._electronSettings);

ipcRenderer.send('settings_get');
}
},

componentDidMount: function() {
Expand All @@ -216,6 +224,15 @@ module.exports = React.createClass({
if (cli) {
cli.removeListener("RoomMember.membership", this._onInviteStateChange);
}

if (PlatformPeg.get().isElectron()) {
const {ipcRenderer} = require('electron');
ipcRenderer.removeListener('settings', this._electronSettings);
}
},

_electronSettings: function(ev, settings) {
this.setState({ electron_settings: settings });
},

_refreshFromServer: function() {
Expand Down Expand Up @@ -787,6 +804,29 @@ module.exports = React.createClass({
</div>;
},

_renderElectronSettings: function() {
const settings = this.state.electron_settings;
if (!settings) return;

const {ipcRenderer} = require('electron');

return <div>
<h3>Electron Settings</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Users don't know what electron is. "Desktop-specific settings" perhaps?

<div className="mx_UserSettings_section">
<div className="mx_UserSettings_toggle">
<input type="checkbox"
name="auto-launch"
defaultChecked={settings['auto-launch']}
onChange={(e) => {
ipcRenderer.send('settings_set', 'auto-launch', e.target.checked);
}}
/>
<label htmlFor="auto-launch">Start automatically after system login</label>
</div>
</div>
</div>;
},

_showSpoiler: function(event) {
const target = event.target;
target.innerHTML = target.getAttribute('data-spoiler');
Expand Down Expand Up @@ -988,6 +1028,8 @@ module.exports = React.createClass({
{this._renderBulkOptions()}
{this._renderBugReport()}

{PlatformPeg.get().isElectron() && this._renderElectronSettings()}

<h3>Advanced</h3>

<div className="mx_UserSettings_section">
Expand Down