From 7137ba71888174adf011eb28a35ec72a5a6fe701 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 25 Sep 2017 14:13:02 +0100 Subject: [PATCH 1/5] Refactor right panel header buttons To reduce repetition of code, simplify the process of adding header buttons and remove the need for many handler functions. --- src/components/structures/RightPanel.js | 138 ++++++++++++++---------- 1 file changed, 81 insertions(+), 57 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index f867cd32fd..ff83e3c9cc 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -17,17 +17,56 @@ limitations under the License. */ import React from 'react'; +import PropTypes from 'prop-types'; import { _t } from 'matrix-react-sdk/lib/languageHandler'; import sdk from 'matrix-react-sdk'; -import Matrix from "matrix-js-sdk"; import dis from 'matrix-react-sdk/lib/dispatcher'; import MatrixClientPeg from 'matrix-react-sdk/lib/MatrixClientPeg'; import Analytics from 'matrix-react-sdk/lib/Analytics'; import rate_limited_func from 'matrix-react-sdk/lib/ratelimitedfunc'; -import Modal from 'matrix-react-sdk/lib/Modal'; import AccessibleButton from 'matrix-react-sdk/lib/components/views/elements/AccessibleButton'; import { showGroupInviteDialog } from 'matrix-react-sdk/lib/GroupInvite'; +class HeaderButton extends React.Component { + constructor() { + super(); + this.onClick = this.onClick.bind(this); + } + + onClick(ev) { + dis.dispatch({ + action: 'view_right_panel_phase', + phase: this.props.clickPhase, + }); + } + + render() { + const TintableSvg = sdk.getComponent("elements.TintableSvg"); + const AccessibleButton = sdk.getComponent("elements.AccessibleButton"); + const isHighlighted = this.props.phases.includes(this.props.currentPhase); + + return +
+ { this.props.badge ? this.props.badge :  } +
+ + { isHighlighted ?
:
} + ; + } +} + +HeaderButton.propTypes = { + phases: PropTypes.arrayOf(PropTypes.string).isRequired, + currentPhase: PropTypes.string.isRequired, + clickPhase: PropTypes.string.isRequired, + iconSrc: PropTypes.string.isRequired, + badge: PropTypes.node, +}; + module.exports = React.createClass({ displayName: 'RightPanel', @@ -140,7 +179,7 @@ module.exports = React.createClass({ } else { if (this.props.roomId) { this.setState({ - phase: this.Phase.RoomMemberList + phase: this.Phase.RoomMemberList, }); } else if (this.props.groupId) { this.setState({ @@ -164,7 +203,11 @@ module.exports = React.createClass({ }); } else if (payload.action === "view_room") { this.setState({ - phase: this.Phase.RoomMemberList + phase: this.Phase.RoomMemberList, + }); + } else if (payload.action === "view_right_panel_phase") { + this.setState({ + phase: payload.phase, }); } }, @@ -176,36 +219,22 @@ module.exports = React.createClass({ const FilePanel = sdk.getComponent('structures.FilePanel'); const TintableSvg = sdk.getComponent("elements.TintableSvg"); let inviteGroup; - let panel; - - let filesHighlight; - let membersHighlight; - let notificationsHighlight; - if (!this.props.collapsed) { - if (this.state.phase == this.Phase.RoomMemberList || this.state.phase === this.Phase.RoomMemberInfo) { - membersHighlight =
; - } - else if (this.state.phase == this.Phase.FilePanel) { - filesHighlight =
; - } - else if (this.state.phase == this.Phase.NotificationPanel) { - notificationsHighlight =
; - } - } let membersBadge; - if ((this.state.phase == this.Phase.RoomMemberList || this.state.phase === this.Phase.RoomMemberInfo) && this.props.roomId) { + if ((this.state.phase == this.Phase.RoomMemberList || this.state.phase === this.Phase.RoomMemberInfo) + && this.props.roomId + ) { const cli = MatrixClientPeg.get(); const room = cli.getRoom(this.props.roomId); - let user_is_in_room; + let userIsInRoom; if (room) { membersBadge = room.getJoinedMembers().length; - user_is_in_room = room.hasMembershipState( - MatrixClientPeg.get().credentials.userId, 'join' + userIsInRoom = room.hasMembershipState( + MatrixClientPeg.get().credentials.userId, 'join', ); } - if (user_is_in_room) { + if (userIsInRoom) { inviteGroup =
@@ -214,37 +243,28 @@ module.exports = React.createClass({
{ _t('Invite to this room') }
; } - } let headerButtons = []; if (this.props.roomId) { - headerButtons.push( - -
{ membersBadge ? membersBadge :  }
- - { membersHighlight } - - ); - headerButtons.push( - -
 
- - { filesHighlight } - - ); - headerButtons.push( - -
 
- - { notificationsHighlight } - - ); + headerButtons = [ + , + , + , + ]; } if (this.props.roomId || this.props.groupId) { @@ -256,13 +276,14 @@ module.exports = React.createClass({ title={ _t("Hide panel") } onClick={ this.onCollapseClick } > -
+
, ); } + let panel =
; if (!this.props.collapsed) { if (this.props.roomId && this.state.phase == this.Phase.RoomMemberList) { - panel = + panel = ; } else if (this.props.groupId && this.state.phase == this.Phase.GroupMemberList) { panel = ; inviteGroup = ( @@ -275,10 +296,13 @@ module.exports = React.createClass({ ); } else if (this.state.phase == this.Phase.RoomMemberInfo) { const MemberInfo = sdk.getComponent('rooms.MemberInfo'); - panel = + panel = ; } else if (this.state.phase == this.Phase.GroupMemberInfo) { const GroupMemberInfo = sdk.getComponent('groups.GroupMemberInfo'); - panel = ; + panel = ; } else if (this.state.phase == this.Phase.NotificationPanel) { panel = ; } else if (this.state.phase == this.Phase.FilePanel) { @@ -308,5 +332,5 @@ module.exports = React.createClass({
); - } + }, }); From 28e9fdc873467765a3aafd7904421b91005af84d Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 25 Sep 2017 14:42:44 +0100 Subject: [PATCH 2/5] rebase in progress; onto 7137ba71 --- src/components/structures/RightPanel.js | 27 +++++++++++-------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index ff83e3c9cc..c58d3c625b 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -34,6 +34,7 @@ class HeaderButton extends React.Component { } onClick(ev) { + Analytics.trackEvent(...this.props.analytics); dis.dispatch({ action: 'view_right_panel_phase', phase: this.props.clickPhase, @@ -60,11 +61,19 @@ class HeaderButton extends React.Component { } HeaderButton.propTypes = { + // If currentPhase is one of the specified phases, the button will be highlighted phases: PropTypes.arrayOf(PropTypes.string).isRequired, + // The currentPhase of the RightPanel currentPhase: PropTypes.string.isRequired, + // The phase to swap to when the button is clicked clickPhase: PropTypes.string.isRequired, + // The source file of the icon to display iconSrc: PropTypes.string.isRequired, + + // The badge to display above the icon badge: PropTypes.node, + // The parameters to track the click event + analytics: PropTypes.arrayOf(PropTypes.string).isRequired, }; module.exports = React.createClass({ @@ -112,21 +121,6 @@ module.exports = React.createClass({ } }, - onMemberListButtonClick: function() { - Analytics.trackEvent('Right Panel', 'Member List Button', 'click'); - this.setState({ phase: this.Phase.RoomMemberList }); - }, - - onFileListButtonClick: function() { - Analytics.trackEvent('Right Panel', 'File List Button', 'click'); - this.setState({ phase: this.Phase.FilePanel }); - }, - - onNotificationListButtonClick: function() { - Analytics.trackEvent('Right Panel', 'Notification List Button', 'click'); - this.setState({ phase: this.Phase.NotificationPanel }); - }, - onCollapseClick: function() { dis.dispatch({ action: 'hide_right_panel', @@ -253,16 +247,19 @@ module.exports = React.createClass({ clickPhase={this.Phase.RoomMemberList} currentPhase={this.state.phase} badge={membersBadge} + analytics={['Right Panel', 'Member List Button', 'click']} />, , , ]; } From fd7e81193eaacb0262eac089ec1bcbcbd64030cf Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 25 Sep 2017 16:06:46 +0100 Subject: [PATCH 3/5] Less coupling between HeaderButton and RightPanel Use isHighlighted as a prop instead of computing based on phases and currentPhase --- src/components/structures/RightPanel.js | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index c58d3c625b..6b7633ad4a 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -44,7 +44,6 @@ class HeaderButton extends React.Component { render() { const TintableSvg = sdk.getComponent("elements.TintableSvg"); const AccessibleButton = sdk.getComponent("elements.AccessibleButton"); - const isHighlighted = this.props.phases.includes(this.props.currentPhase); return  } - { isHighlighted ?
:
} + { this.props.isHighlighted ?
:
} ; } } HeaderButton.propTypes = { - // If currentPhase is one of the specified phases, the button will be highlighted - phases: PropTypes.arrayOf(PropTypes.string).isRequired, - // The currentPhase of the RightPanel - currentPhase: PropTypes.string.isRequired, + // Whether this button is highlighted + isHighlighted: PropTypes.bool.isRequired, // The phase to swap to when the button is clicked clickPhase: PropTypes.string.isRequired, // The source file of the icon to display @@ -243,22 +240,18 @@ module.exports = React.createClass({ if (this.props.roomId) { headerButtons = [ , , , ]; From 77f30aacf99cbea81cd4d30c445ad73ac196d69a Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 25 Sep 2017 16:13:13 +0100 Subject: [PATCH 4/5] Re-add clickPhase for FilePanel header button --- src/components/structures/RightPanel.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index 6b7633ad4a..be547ca7e1 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -246,6 +246,7 @@ module.exports = React.createClass({ analytics={['Right Panel', 'Member List Button', 'click']} />, , From bfdb92ff6b87caf7762f1379eae8179087d85c1a Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 25 Sep 2017 16:13:54 +0100 Subject: [PATCH 5/5] Nit: Re-order property --- src/components/structures/RightPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index be547ca7e1..0a438881ff 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -246,8 +246,8 @@ module.exports = React.createClass({ analytics={['Right Panel', 'Member List Button', 'click']} />, ,