Implement a store for RoomView #921
Conversation
This allows for a truely flux-y way of storing the currently viewed room, making some callbacks (like onRoomIdResolved) redundant and making sure that the currently viewed room (ID) is only stored in one place as opposed to the previous many places.
This was required for the `join_room` action which can be dispatched to join the currently viewed room.
Another change was to introduce `LifeCycleStore` which is a start at encorporating state related to the lifecycle of the app into a flux store. Currently it only contains an action which will be dispatched when the sync state has become PREPARED. This was necessary to do a deferred dispatch of `join_room` following the registration of a PWLU (PassWord-Less User).
The following actions are introduced:
- RoomViewStore:
- `view_room`: dispatch to change the currently viewed room ID
- `join_room`: dispatch to join the currently viewed room
- LifecycleStore:
- `do_after_sync_prepared`: dispatch to store an action which will be dispatched when `sync_state` is dispatched with `state = 'PREPARED'`
- MatrixChat:
- `sync_state`: dispatched when the sync state changes. Ideally there'd be a SyncStateStore that emitted an `update` upon receiving this, but for now the `LifecycleStore` will listen for `sync_state` directly.
There was a problem hiding this comment.
In general this looks OK. It's nice to have the functional bits split out of roomview. Have you tested:
- navigating to a room alias via the URL bar (does the right alias get to the room preview bar?)
- Clicking a room from the room directory (name + room icon should be displayed in room preview bar)
| onRegistered={this.props.onRegistered} | ||
| eventId={this.props.initialEventId} | ||
| thirdPartyInvite={this.props.thirdPartyInvite} | ||
| oobData={this.props.roomOobData} | ||
| highlightedEventId={this.props.highlightedEventId} | ||
| eventPixelOffset={this.props.initialEventPixelOffset} | ||
| key={this.props.currentRoomAlias || this.props.currentRoomId} | ||
| key={this.props.currentRoomId} |
There was a problem hiding this comment.
Yep. As there's only one RoomView, I'll just give it a default key of "roomview".
There was a problem hiding this comment.
ok - is this going to break the assumption that a fresh roomview will always be created for a new room if you navigate from one alias to the other (which I think is when the room id is null)?
src/stores/LifecycleStore.js
Outdated
| if (payload.state !== 'PREPARED') { | ||
| break; | ||
| } | ||
| console.warn(this._state); |
| @@ -891,6 +895,7 @@ module.exports = React.createClass({ | |||
| }); | |||
|
|
|||
| cli.on('sync', function(state, prevState) { | |||
| dis.dispatch({action: 'sync_state', prevState, state}); | |||
There was a problem hiding this comment.
I don't really get why it's MatrixChat's job to relay this between the client and the store: if the store cares, shouldn't it just subscribe?
There was a problem hiding this comment.
The good thing about flux.Store is that it enforces the rule that everything that calls __emitChange (which is done after _setState) must be done during a dispatch. The js-sdk is not a dispatcher so on.('sync', cb) doesn't count and cb cannot cause the store to update.
So the store can't subscribe. As per my commit message, ideally there'd be a sync state store which registered as a listener and dispatched to itself a sync_state which would then propagate the actual sync state.
There was a problem hiding this comment.
Eugh, sounds like the general impedance mismatch of the two different dispatch mechanisms, but also sounds like this is the correct way to get around it. A comment might not be a bad idea so you don't have to have this conversation with the next 8 people who read that line.
There was a problem hiding this comment.
Good idea, I've added a comment.
|
otherwise lgtm |
|
I have tested that functionality btw, forgot to mention. |
This allows for a truely flux-y way of storing the currently viewed room, making some callbacks (like onRoomIdResolved) redundant and making sure that the currently viewed room (ID) is only stored in one place as opposed to the previous many places.
This was required for the
join_roomaction which can be dispatched to join the currently viewed room. Fixes element-hq/element-web#3986Another change was to introduce
LifeCycleStorewhich is a start at encorporating state related to the lifecycle of the app into a flux store. Currently it only contains an action which will be dispatched when the sync state has become PREPARED. This was necessary to do a deferred dispatch ofjoin_roomfollowing the registration of a PWLU (PassWord-Less User).The following actions are introduced:
view_room: dispatch to change the currently viewed room IDjoin_room: dispatch to join the currently viewed roomdo_after_sync_prepared: dispatch to store an action which will be dispatched whensync_stateis dispatched withstate = 'PREPARED'sync_state: dispatched when the sync state changes. Ideally there'd be a SyncStateStore that emitted anupdateupon receiving this, but for now theLifecycleStorewill listen forsync_statedirectly.