-
-
Notifications
You must be signed in to change notification settings - Fork 827
DnD Ordered TagPanel #1653
DnD Ordered TagPanel #1653
Changes from 9 commits
8178496
82a95f0
a8a650c
7aa5dce
4af7def
35a108e
a9cc8eb
8f88995
7e1f1cd
65d8833
ee6df10
1251544
7255096
31a52c1
53e7232
8f07744
df88b71
991ea4e
aa91409
0b38bf5
8d2d3e6
a120335
3e532e3
60d8ebb
13925db
d5534a9
cc30b8f
5de0559
42c1f3c
a8b245d
f38690f
e1ea8f0
a653ece
ddf5dba
31ea092
fe6b7c0
950f591
6b02f59
629cd13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
Copyright 2017 Vector Creations Ltd | ||
Copyright 2017 New Vector Ltd | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
|
@@ -18,6 +18,14 @@ import { asyncAction } from './actionCreators'; | |
|
||
const GroupActions = {}; | ||
|
||
/** | ||
* Create a GroupActions.fetchJoinedGroups action that represents an | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is still incorrect |
||
* asyncronous request to fetch the groups to which a user is joined. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. asynchronous |
||
* | ||
* @param {MatrixClient} matrixClient the matrix client to query. | ||
* @returns {function} an asyncronous action of type | ||
* GroupActions.fetchJoinedGroups. | ||
*/ | ||
GroupActions.fetchJoinedGroups = function(matrixClient) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs more docs |
||
return asyncAction('GroupActions.fetchJoinedGroups', () => matrixClient.getJoinedGroups()); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
Copyright 2017 Vector Creations Ltd | ||
Copyright 2017 New Vector Ltd | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
|
@@ -19,8 +19,7 @@ import dis from '../dispatcher'; | |
// TODO: migrate from sync_state to MatrixActions.sync so that more js-sdk events | ||
// become dispatches in the same place. | ||
/** | ||
* An action creator that will map a `sync` event to a MatrixActions.sync action, | ||
* each parameter mapping to a key-value in the action. | ||
* Create a MatrixActions.sync action that represents a MatrixClient `sync` event. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, we should still document the members of the action object imho. |
||
* | ||
* @param {MatrixClient} matrixClient the matrix client | ||
* @param {string} state the current sync state. | ||
|
@@ -37,11 +36,10 @@ function createSyncAction(matrixClient, state, prevState) { | |
} | ||
|
||
/** | ||
* An action creator that will map an account data matrix event to a | ||
* MatrixActions.accountData action. | ||
* Create a MatrixActions.accountData action that represents a MatrixClient `accountData` | ||
* matrix event. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (it would help my brain if you could give some examples of what |
||
* | ||
* @param {MatrixClient} matrixClient the matrix client with which to | ||
* register a listener. | ||
* @param {MatrixClient} matrixClient the matrix client. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we pass this in, given it's ignored? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically so that we can handle both action creators the same in |
||
* @param {MatrixEvent} accountDataEvent the account data event. | ||
* @returns {Object} an action of type MatrixActions.accountData. | ||
*/ | ||
|
@@ -54,14 +52,33 @@ function createAccountDataAction(matrixClient, accountDataEvent) { | |
}; | ||
} | ||
|
||
/** | ||
* This object is responsible for dispatching actions when certain events are emitted by | ||
* the given MatrixClient. | ||
*/ | ||
export default { | ||
// A list of callbacks to call to unregister all listeners added | ||
_matrixClientListenersStop: [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could probably do with a comment here too |
||
|
||
/** | ||
* Start listening to certain events from the MatrixClient and dispatch actions when | ||
* they are emitted. | ||
* @param {MatrixClient} matrixClient the MatrixClient to listen to events from | ||
*/ | ||
start(matrixClient) { | ||
this._addMatrixClientListener(matrixClient, 'sync', createSyncAction); | ||
this._addMatrixClientListener(matrixClient, 'accountData', createAccountDataAction); | ||
}, | ||
|
||
/** | ||
* Start listening to events emitted by matrixClient, dispatch an action created by the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a word missing here? |
||
* actionCreator function. | ||
* @param {MatrixClient} matrixClient a MatrixClient to register a listener with. | ||
* @param {string} eventName the event to listen to on MatrixClient. | ||
* @param {function} actionCreator a function that should return an action to dispatch | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you also specify that the function will be passed the MatrixClient event? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming you meant MatrixClient itself There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. both, but yes. |
||
* when given the arguments emitted in the MatrixClient | ||
* event. | ||
*/ | ||
_addMatrixClientListener(matrixClient, eventName, actionCreator) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is internal, but it's opaque enough that it could do with some comments |
||
const listener = (...args) => { | ||
dis.dispatch(actionCreator(matrixClient, ...args)); | ||
|
@@ -72,6 +89,9 @@ export default { | |
}); | ||
}, | ||
|
||
/** | ||
* Stop listening to events. | ||
*/ | ||
stop() { | ||
this._matrixClientListenersStop.forEach((stopListener) => stopListener()); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
Copyright 2017 Vector Creations Ltd | ||
Copyright 2017 New Vector Ltd | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
|
@@ -20,6 +20,16 @@ import TagOrderStore from '../stores/TagOrderStore'; | |
|
||
const TagOrderActions = {}; | ||
|
||
/** | ||
* Create a TagOrderActions.commitTagOrdering action that represents an | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as with GroupActions.fetchJoinedGroups, I don't think this is correct. |
||
* asyncronous request to commit TagOrderStore.getOrderedTags() to account | ||
* data. | ||
* | ||
* @param {MatrixClient} matrixClient the matrix client to set the account | ||
* data on. | ||
* @returns {function} an asyncronous action of type | ||
* TagOrderActions.commitTagOrdering. | ||
*/ | ||
TagOrderActions.commitTagOrdering = function(matrixClient) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs docs |
||
return asyncAction('TagOrderActions.commitTagOrdering', () => { | ||
// Only commit tags if the state is ready, i.e. not null | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,15 @@ const TagPanel = React.createClass({ | |
|
||
getInitialState() { | ||
return { | ||
orderedGroupTagProfiles: [], | ||
// A list of group profiles for group tags | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't mean a great deal to me. Probably merits a longer explanation. |
||
orderedGroupTagProfiles: [ | ||
// { | ||
// groupId: '+awesome:foo.bar',{ | ||
// name: 'My Awesome Community', | ||
// avatarUrl: 'mxc://...', | ||
// shortDescription: 'Some description...', | ||
// }, | ||
], | ||
selectedTags: [], | ||
}; | ||
}, | ||
|
@@ -64,6 +72,7 @@ const TagPanel = React.createClass({ | |
Promise.all(orderedGroupTags.map( | ||
(groupId) => FlairStore.getGroupProfileCached(this.context.matrixClient, groupId), | ||
)).then((orderedGroupTagProfiles) => { | ||
if (this.unmounted) return; | ||
this.setState({orderedGroupTagProfiles}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this probably needs an unmounted guard? |
||
}); | ||
}); | ||
|
@@ -81,7 +90,7 @@ const TagPanel = React.createClass({ | |
|
||
_onGroupMyMembership() { | ||
if (this.unmounted) return; | ||
dis.dispatch(GroupActions.fetchJoinedGroups.bind(this.context.matrixClient)); | ||
dis.dispatch(GroupActions.fetchJoinedGroups(this.context.matrixClient)); | ||
}, | ||
|
||
onClick() { | ||
|
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 aiui.
It returns an action creator: ie, a function which, when called, will start such a request and create such a (set of) actions.