[client-app] Fix measurement of thread-list action metrics

Summary:
We detect when a thread is removed from the thread-list by listening to
`Actions.threadListDidUpdate`. However, we were not firing the action at
the correct moment.

Before this commit, we fired the action on the root `ThreadList`'s
`componentDidUpdate`, however did this not actually fire when the
child list actually updated/removed threads from the list.

This sort of used to work because before 396a027bcb
the root ThreadList component re-rendered all the time, so it fired the
action, but after initial sync, we would never actually report any
thread-list action metrics at all

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: halla, evan

Differential Revision: https://phab.nylas.com/D4051
This commit is contained in:
Juan Tejada 2017-02-28 13:55:04 -08:00
parent fa17d2f1e8
commit b6a1fc5234
3 changed files with 14 additions and 5 deletions

View file

@ -57,11 +57,6 @@ class ThreadList extends React.Component
(not Utils.isEqualReact(@state, nextState))
)
componentDidUpdate: =>
dataSource = ThreadListStore.dataSource()
threads = dataSource.itemsCurrentlyInView()
Actions.threadListDidUpdate(threads)
componentWillUnmount: =>
@unsub()
window.removeEventListener('resize', @_onResize, true)
@ -137,10 +132,16 @@ class ThreadList extends React.Component
onDoubleClick={(thread) -> Actions.popoutThread(thread)}
onDragStart={@_onDragStart}
onDragEnd={@_onDragEnd}
onComponentDidUpdate={@_onThreadListDidUpdate}
/>
</FocusContainer>
</FluxContainer>
_onThreadListDidUpdate: =>
dataSource = ThreadListStore.dataSource()
threads = dataSource.itemsCurrentlyInView()
Actions.threadListDidUpdate(threads)
_threadPropsProvider: (item) ->
classes = classnames({
'unread': item.unread

View file

@ -97,6 +97,7 @@ class ListTabular extends Component {
onDoubleClick: PropTypes.func,
onDragStart: PropTypes.func,
onDragEnd: PropTypes.func,
onComponentDidUpdate: PropTypes.func,
};
static defaultProps = {
@ -133,6 +134,9 @@ class ListTabular extends Component {
}
componentDidUpdate(prevProps) {
if (this.props.onComponentDidUpdate) {
this.props.onComponentDidUpdate()
}
// If our view has been swapped out for an entirely different one,
// reset our scroll position to the top.
if (prevProps.dataSource !== this.props.dataSource) {

View file

@ -34,6 +34,7 @@ class MultiselectList extends React.Component
columns: React.PropTypes.array.isRequired
itemPropsProvider: React.PropTypes.func.isRequired
keymapHandlers: React.PropTypes.object
onComponentDidUpdate: React.PropTypes.func
constructor: (@props) ->
@state = @_getStateFromStores()
@ -48,6 +49,8 @@ class MultiselectList extends React.Component
@setState(@_getStateFromStores(newProps))
componentDidUpdate: (prevProps, prevState) =>
if @props.onComponentDidUpdate
@props.onComponentDidUpdate()
if prevProps.focusedId isnt @props.focusedId or
prevProps.keyboardCursorId isnt @props.keyboardCursorId
@ -120,6 +123,7 @@ class MultiselectList extends React.Component
dataSource={@props.dataSource}
itemPropsProvider={@itemPropsProvider}
onSelect={@_onClickItem}
onComponentDidUpdate={@props.onComponentDidUpdate}
{...otherProps} />
</KeyCommandsRegion>
else