Move manage integrations button from settings page to room header as a stand-alone component #1286
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| /* | ||
| Copyright 2017 Vector Creations Ltd | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import sdk from '../../../index'; | ||
| import SdkConfig from '../../../SdkConfig'; | ||
| import ScalarAuthClient from '../../../ScalarAuthClient'; | ||
| import ScalarMessaging from '../../../ScalarMessaging'; | ||
| import Modal from "../../../Modal"; | ||
| import { _t } from '../../../languageHandler'; | ||
| import AccessibleButton from './AccessibleButton'; | ||
| import TintableSvg from './TintableSvg'; | ||
|
|
||
| export default class ManageIntegsButton extends React.Component { | ||
| constructor(props) { | ||
| super(props); | ||
|
|
||
| this.state = { | ||
| scalar_error: null, | ||
|
|
||
| showIntegrationsError: false, | ||
| }; | ||
|
|
||
| this.onManageIntegrations = this.onManageIntegrations.bind(this); | ||
| this.onShowIntegrationsError = this.onShowIntegrationsError.bind(this); | ||
| } | ||
|
|
||
| componentWillMount() { | ||
| ScalarMessaging.startListening(); | ||
| this.scalarClient = null; | ||
|
|
||
| if (SdkConfig.get().integrations_ui_url && SdkConfig.get().integrations_rest_url) { | ||
|
There was a problem hiding this comment. It's strange that we don't pass these URLs into the auth client here, and have to check the config before creating a ScakarAuthClient. There was a problem hiding this comment. Not an argument against fixing it, but this is simply copied across from the existing code on the settings page -- https://github.com/matrix-org/matrix-react-sdk/pull/1286/files#diff-7810353d2ee5218b7242e81423030433L141. Unless you think this is broken, I'm inclined to leave it. What do you reckon? There was a problem hiding this comment. It's not broken, it's just a weird API that made sense when we only ever made one Scalar client. Feel free to leave it for now. There was a problem hiding this comment. I wrote this check to hide all instances of Manage Integrations/Scalar/Modular if there is are no integrations URLs defined in the config. i.e a way to hide the buttons. This seems to have regressed from the original function: #816 |
||
| this.scalarClient = new ScalarAuthClient(); | ||
| this.scalarClient.connect().done(() => { | ||
| this.forceUpdate(); | ||
| }, (err) => { | ||
| this.setState({ scalar_error: err}); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| componentWillUnmount() { | ||
| ScalarMessaging.stopListening(); | ||
| } | ||
|
|
||
| onManageIntegrations(ev) { | ||
| ev.preventDefault(); | ||
| const IntegrationsManager = sdk.getComponent("views.settings.IntegrationsManager"); | ||
| Modal.createDialog(IntegrationsManager, { | ||
| src: (this.scalarClient !== null && this.scalarClient.hasCredentials()) ? | ||
| this.scalarClient.getScalarInterfaceUrlForRoom(this.props.room.roomId) : | ||
| null, | ||
|
There was a problem hiding this comment. Would this not open an iFrame with a There was a problem hiding this comment. As with some of the previous comments, this has just been cargo-culted across from the existing room settings code (https://github.com/matrix-org/matrix-react-sdk/pull/1286/files#diff-7810353d2ee5218b7242e81423030433L520). As it is not currently considered broken, I'm inclined to leave it as is, for now. |
||
| onFinished: ()=>{ | ||
| if (this.props.onCancelClick) { | ||
| this.props.onCancelClick(ev); | ||
|
There was a problem hiding this comment. You probably don't want to be returning this There was a problem hiding this comment. As with one of the previous comments, this code is directly copied across from the existing settings page - https://github.com/matrix-org/matrix-react-sdk/pull/1286/files#diff-7810353d2ee5218b7242e81423030433L525. I'm inclined to leave it, for fear of breaking existing expected behaviour. There was a problem hiding this comment.
This property is never set - the entire This means that the behaviour has changed, given you've copied it. Oh! I guess this is the thing where if you close the integrations manager, you also close the room settings. Maybe that wasn't so bizarre after all? There was a problem hiding this comment. As discussed IRL, I've removed this all together from the ManageIntegsButton component, as it doesn't appear to be used at the moment. |
||
| } | ||
| }, | ||
| }, "mx_IntegrationsManager"); | ||
| } | ||
|
|
||
| onShowIntegrationsError(ev) { | ||
| ev.preventDefault(); | ||
| this.setState({ | ||
| showIntegrationsError: !this.state.showIntegrationsError, | ||
| }); | ||
| } | ||
|
|
||
| render() { | ||
| let integrationsButton; | ||
| let integrationsError; | ||
| if (this.scalarClient !== null) { | ||
| if (this.state.showIntegrationsError && this.state.scalar_error) { | ||
| console.error(this.state.scalar_error); | ||
|
There was a problem hiding this comment. This should probably be logged after line 50 |
||
| integrationsError = ( | ||
| <span className="mx_RoomSettings_integrationsButton_errorPopup"> | ||
| { _t('Could not connect to the integration server') } | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| if (this.scalarClient.hasCredentials()) { | ||
| integrationsButton = ( | ||
| <AccessibleButton className="mx_RoomHeader_button" onClick={this.onManageIntegrations} title={ _t('Manage Integrations') }> | ||
| <TintableSvg src="img/icons-apps.svg" width="35" height="35"/> | ||
| </AccessibleButton> | ||
| ); | ||
| } else if (this.state.scalar_error) { | ||
| integrationsButton = ( | ||
| <div className="mx_RoomSettings_integrationsButton_error" onClick={ this.onShowIntegrationsError }> | ||
| <img src="img/warning.svg" title={_t('Integrations Error')} width="17"/> | ||
| { integrationsError } | ||
| </div> | ||
| ); | ||
| } else { | ||
| integrationsButton = ( | ||
| <AccessibleButton className="mx_RoomHeader_button" onClick={this.onManageIntegrations} title={ _t('Manage Integrations') }> | ||
| <TintableSvg src="img/icons-apps.svg" width="35" height="35"/> | ||
| </AccessibleButton> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return integrationsButton; | ||
| } | ||
| } | ||
|
|
||
| ManageIntegsButton.propTypes = { | ||
| room: PropTypes.object.isRequired, | ||
|
There was a problem hiding this comment. I'd prefer a |
||
| onCancelClick: PropTypes.func, | ||
| }; | ||
|
|
||
| ManageIntegsButton.defaultProps = { | ||
|
|
||
| onCancelClick: function() {}, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ import * as linkify from 'linkifyjs'; | |
| import linkifyElement from 'linkifyjs/element'; | ||
| import linkifyMatrix from '../../../linkify-matrix'; | ||
| import AccessibleButton from '../elements/AccessibleButton'; | ||
| import ManageIntegsButton from '../elements/ManageIntegsButton'; | ||
| import {CancelButton} from './SimpleRoomHeader'; | ||
|
|
||
| linkifyMatrix(linkify); | ||
|
|
@@ -47,13 +48,15 @@ module.exports = React.createClass({ | |
| onSaveClick: React.PropTypes.func, | ||
| onSearchClick: React.PropTypes.func, | ||
| onLeaveClick: React.PropTypes.func, | ||
| onCancelClick: React.PropTypes.func, | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. Although it is not used by anything that I have added, this prop actually is used elsewhere in the code... https://github.com/matrix-org/matrix-react-sdk/pull/1286/files#diff-96b79748dd7ebec5d36339d2478e92cbR194 I was adding it in to the component props and default to make it clear to future people (like me) that this prop is passed in are available to be used by other components and to make sure that there is a sensible default if the function is not passed. I can remove it if you want, but I think it makes more sense to leave it? There was a problem hiding this comment. My mistake, this looks to be necessary. I think I was confused by the fact that it wasn't in prop types previously. |
||
| }, | ||
|
|
||
| getDefaultProps: function() { | ||
| return { | ||
| editing: false, | ||
| inRoom: false, | ||
| onSaveClick: function() {}, | ||
| onCancelClick: function() {}, | ||
| }; | ||
| }, | ||
|
|
||
|
|
@@ -320,10 +323,15 @@ module.exports = React.createClass({ | |
| } | ||
|
|
||
| let rightRow; | ||
|
|
||
| if (!this.props.editing) { | ||
| rightRow = | ||
| <div className="mx_RoomHeader_rightRow"> | ||
| { settingsButton } | ||
| <ManageIntegsButton | ||
| onCancelClick={this.props.onCancelClick} | ||
|
There was a problem hiding this comment. It seems quite bizarre to be bubbling the There was a problem hiding this comment. Thanks for spotting that! |
||
| room={this.props.room} | ||
| /> | ||
| { forgetButton } | ||
| { searchButton } | ||
| { rightPanelButtons } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not anymore!