From 775468e71a89288ac76fd657413f1b10514cb907 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 31 Oct 2017 11:42:09 +0000 Subject: [PATCH 1/3] Display whether the group summary/room list is loading This uses a `ready` flag assigned to each fetching API used by the GroupServer. I've avoided making this generic for now for want of not doing so early. --- src/components/structures/GroupView.js | 11 +++++++++-- src/components/views/rooms/RoomDetailList.js | 6 ++++++ src/stores/GroupStore.js | 13 +++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index fa75df53f0..76f6bc7335 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -407,6 +407,10 @@ export default React.createClass({ getInitialState: function() { return { summary: null, + isGroupPublicised: null, + isUserPrivileged: null, + groupRooms: null, + groupRoomsLoading: null, error: null, editing: false, saving: false, @@ -458,8 +462,11 @@ export default React.createClass({ } this.setState({ summary, + summaryLoading: !this._groupStore.isStateReady('GroupStore.Summary'), isGroupPublicised: this._groupStore.getGroupPublicity(), isUserPrivileged: this._groupStore.isUserPrivileged(), + groupRooms: this._groupStore.getGroupRooms(), + groupRoomsLoading: !this._groupStore.isStateReady('GroupStore.GroupRooms'), error: null, }); }); @@ -667,7 +674,7 @@ export default React.createClass({

{ _t('Rooms') }

{ addRoomRow } - + ; }, @@ -863,7 +870,7 @@ export default React.createClass({ const Spinner = sdk.getComponent("elements.Spinner"); const TintableSvg = sdk.getComponent("elements.TintableSvg"); - if (this.state.summary === null && this.state.error === null || this.state.saving) { + if (this.state.summaryLoading && this.state.error === null || this.state.saving) { return ; } else if (this.state.summary) { const summary = this.state.summary; diff --git a/src/components/views/rooms/RoomDetailList.js b/src/components/views/rooms/RoomDetailList.js index be9de849e9..cdd05d1698 100644 --- a/src/components/views/rooms/RoomDetailList.js +++ b/src/components/views/rooms/RoomDetailList.js @@ -113,6 +113,8 @@ export default React.createClass({ worldReadable: PropTypes.bool, guestCanJoin: PropTypes.bool, + + loading: PropTypes.bool, })), }, @@ -124,6 +126,10 @@ export default React.createClass({ }, render() { + const Spinner = sdk.getComponent('elements.Spinner'); + if (this.props.loading) { + return ; + } const rows = this.getRows(); let rooms; if (rows.length == 0) { diff --git a/src/stores/GroupStore.js b/src/stores/GroupStore.js index 66bc293b44..d3c514f489 100644 --- a/src/stores/GroupStore.js +++ b/src/stores/GroupStore.js @@ -29,6 +29,9 @@ export default class GroupStore extends EventEmitter { this._matrixClient = matrixClient; this._summary = {}; this._rooms = []; + this._members = []; + this._invitedMembers = []; + this._ready = {}; this.on('error', (err) => { console.error(`GroupStore for ${this.groupId} encountered error`, err); @@ -40,6 +43,7 @@ export default class GroupStore extends EventEmitter { this._members = result.chunk.map((apiMember) => { return groupMemberFromApiObject(apiMember); }); + this._ready['GroupStore.GroupMembers'] = true; this._notifyListeners(); }).catch((err) => { console.error("Failed to get group member list: " + err); @@ -50,6 +54,7 @@ export default class GroupStore extends EventEmitter { this._invitedMembers = result.chunk.map((apiMember) => { return groupMemberFromApiObject(apiMember); }); + this._ready['GroupStore.GroupInvitedMembers'] = true; this._notifyListeners(); }).catch((err) => { // Invited users not visible to non-members @@ -64,6 +69,7 @@ export default class GroupStore extends EventEmitter { _fetchSummary() { this._matrixClient.getGroupSummary(this.groupId).then((resp) => { this._summary = resp; + this._ready['GroupStore.Summary'] = true; this._notifyListeners(); }).catch((err) => { this.emit('error', err); @@ -75,6 +81,7 @@ export default class GroupStore extends EventEmitter { this._rooms = resp.chunk.map((apiRoom) => { return groupRoomFromApiObject(apiRoom); }); + this._ready['GroupStore.GroupRooms'] = true; this._notifyListeners(); }).catch((err) => { this.emit('error', err); @@ -87,6 +94,8 @@ export default class GroupStore extends EventEmitter { registerListener(fn) { this.on('update', fn); + // Call to set initial state (before fetching starts) + this.emit('update'); this._fetchSummary(); this._fetchRooms(); this._fetchMembers(); @@ -96,6 +105,10 @@ export default class GroupStore extends EventEmitter { this.removeListener('update', fn); } + isStateReady(id) { + return this._ready[id]; + } + getSummary() { return this._summary; } From d6cbc44e0f4c574c037c599765a3e260254137dc Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 31 Oct 2017 14:21:00 +0000 Subject: [PATCH 2/3] If groupRoomsLoading, replace RoomDetailList entirely with Spinner --- src/components/structures/GroupView.js | 6 +++++- src/components/views/rooms/RoomDetailList.js | 6 ------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index 76f6bc7335..83e0ad8184 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -657,6 +657,7 @@ export default React.createClass({ const RoomDetailList = sdk.getComponent('rooms.RoomDetailList'); const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); const TintableSvg = sdk.getComponent('elements.TintableSvg'); + const Spinner = sdk.getComponent('elements.Spinner'); const addRoomRow = this.state.editing ? ({ _t('Rooms') } { addRoomRow } - + { this.state.groupRoomsLoading ? + : + + } ; }, diff --git a/src/components/views/rooms/RoomDetailList.js b/src/components/views/rooms/RoomDetailList.js index cdd05d1698..be9de849e9 100644 --- a/src/components/views/rooms/RoomDetailList.js +++ b/src/components/views/rooms/RoomDetailList.js @@ -113,8 +113,6 @@ export default React.createClass({ worldReadable: PropTypes.bool, guestCanJoin: PropTypes.bool, - - loading: PropTypes.bool, })), }, @@ -126,10 +124,6 @@ export default React.createClass({ }, render() { - const Spinner = sdk.getComponent('elements.Spinner'); - if (this.props.loading) { - return ; - } const rows = this.getRows(); let rooms; if (rows.length == 0) { From 16dca08b77be54130da8557063b169d9855ee007 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 31 Oct 2017 16:13:13 +0000 Subject: [PATCH 3/3] Use constants instead of string literals --- src/components/structures/GroupView.js | 4 ++-- src/stores/GroupStore.js | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index 1835325ded..24aa552890 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -462,11 +462,11 @@ export default React.createClass({ } this.setState({ summary, - summaryLoading: !this._groupStore.isStateReady('GroupStore.Summary'), + summaryLoading: !this._groupStore.isStateReady(GroupStore.STATE_KEY.Summary), isGroupPublicised: this._groupStore.getGroupPublicity(), isUserPrivileged: this._groupStore.isUserPrivileged(), groupRooms: this._groupStore.getGroupRooms(), - groupRoomsLoading: !this._groupStore.isStateReady('GroupStore.GroupRooms'), + groupRoomsLoading: !this._groupStore.isStateReady(GroupStore.STATE_KEY.GroupRooms), isUserMember: this._groupStore.getGroupMembers().some( (m) => m.userId === MatrixClientPeg.get().credentials.userId, ), diff --git a/src/stores/GroupStore.js b/src/stores/GroupStore.js index d3c514f489..3afac3c049 100644 --- a/src/stores/GroupStore.js +++ b/src/stores/GroupStore.js @@ -23,6 +23,14 @@ import FlairStore from './FlairStore'; * other useful group APIs that may have an effect on the group summary. */ export default class GroupStore extends EventEmitter { + + static STATE_KEY = { + GroupMembers: 'GroupMembers', + GroupInvitedMembers: 'GroupInvitedMembers', + Summary: 'Summary', + GroupRooms: 'GroupRooms', + }; + constructor(matrixClient, groupId) { super(); this.groupId = groupId; @@ -43,7 +51,7 @@ export default class GroupStore extends EventEmitter { this._members = result.chunk.map((apiMember) => { return groupMemberFromApiObject(apiMember); }); - this._ready['GroupStore.GroupMembers'] = true; + this._ready[GroupStore.STATE_KEY.GroupMembers] = true; this._notifyListeners(); }).catch((err) => { console.error("Failed to get group member list: " + err); @@ -54,7 +62,7 @@ export default class GroupStore extends EventEmitter { this._invitedMembers = result.chunk.map((apiMember) => { return groupMemberFromApiObject(apiMember); }); - this._ready['GroupStore.GroupInvitedMembers'] = true; + this._ready[GroupStore.STATE_KEY.GroupInvitedMembers] = true; this._notifyListeners(); }).catch((err) => { // Invited users not visible to non-members @@ -69,7 +77,7 @@ export default class GroupStore extends EventEmitter { _fetchSummary() { this._matrixClient.getGroupSummary(this.groupId).then((resp) => { this._summary = resp; - this._ready['GroupStore.Summary'] = true; + this._ready[GroupStore.STATE_KEY.Summary] = true; this._notifyListeners(); }).catch((err) => { this.emit('error', err); @@ -81,7 +89,7 @@ export default class GroupStore extends EventEmitter { this._rooms = resp.chunk.map((apiRoom) => { return groupRoomFromApiObject(apiRoom); }); - this._ready['GroupStore.GroupRooms'] = true; + this._ready[GroupStore.STATE_KEY.GroupRooms] = true; this._notifyListeners(); }).catch((err) => { this.emit('error', err);