From c09d6dba770ff823f523d375403a119029341e8b Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Mon, 30 Mar 2015 18:08:38 -0700 Subject: [PATCH] fix(animations): Don't process API data while animations are in-flight Summary: Expose the animation coordinator in Nilas-exports, use in more places Provide a way for queries to individually bypass waiting (messages query when opening thread) Test Plan: No new tests to see here yet Reviewers: evan Reviewed By: evan Differential Revision: https://review.inboxapp.com/D1376 --- exports/inbox-exports.coffee | 2 + .../lib/internal-admin-store.coffee | 40 ++++++++-------- src/components/resizable-region.cjsx | 14 +++++- src/components/timeout-transition-group.cjsx | 47 +++++++++++++++---- src/flux/edgehill-api.coffee | 12 +++-- src/flux/inbox-api.coffee | 27 ++++++----- src/flux/models/query.coffee | 7 +++ src/flux/stores/database-store.coffee | 25 ++++++---- src/flux/stores/message-store.coffee | 1 + src/priority-ui-coordinator.coffee | 43 +++++++++++++++++ 10 files changed, 163 insertions(+), 55 deletions(-) create mode 100644 src/priority-ui-coordinator.coffee diff --git a/exports/inbox-exports.coffee b/exports/inbox-exports.coffee index c4d3d26f7..b9b2786ce 100644 --- a/exports/inbox-exports.coffee +++ b/exports/inbox-exports.coffee @@ -37,6 +37,8 @@ module.exports = # Mixins UndoManager: require '../src/flux/undo-manager' + PriorityUICoordinator: require '../src/priority-ui-coordinator' + # Stores DraftStore: require '../src/flux/stores/draft-store' ThreadStore: require '../src/flux/stores/thread-store' diff --git a/internal_packages/sidebar-inbox-internal/lib/internal-admin-store.coffee b/internal_packages/sidebar-inbox-internal/lib/internal-admin-store.coffee index 7f7afc99e..a8a6cba41 100644 --- a/internal_packages/sidebar-inbox-internal/lib/internal-admin-store.coffee +++ b/internal_packages/sidebar-inbox-internal/lib/internal-admin-store.coffee @@ -1,7 +1,7 @@ _ = require 'underscore-plus' Reflux = require 'reflux' request = require 'request' -{FocusedContactsStore, NamespaceStore} = require 'inbox-exports' +{FocusedContactsStore, NamespaceStore, PriorityUICoordinator} = require 'inbox-exports' module.exports = FullContactStore = Reflux.createStore @@ -55,26 +55,28 @@ FullContactStore = Reflux.createStore console.log('Fetching Internal Admin Data') # Swap the url's to see real data request 'https://admin.inboxapp.com/api/status/accounts', (err, resp, data) => - if err - @_error = err - else - @_error = null - try - @_accountCache = JSON.parse(data) - catch err + PriorityUICoordinator.settle.then => + if err @_error = err - @_accountCache = null - @trigger(@) + else + @_error = null + try + @_accountCache = JSON.parse(data) + catch err + @_error = err + @_accountCache = null + @trigger(@) # Swap the url's to see real data request 'https://admin.inboxapp.com/api/status/accounts/applications', (err, resp, data) => - if err - @_error = err - else - @_error = null - try - @_applicationCache = JSON.parse(data) - catch err + PriorityUICoordinator.settle.then => + if err @_error = err - @_applicationCache = null - @trigger(@) \ No newline at end of file + else + @_error = null + try + @_applicationCache = JSON.parse(data) + catch err + @_error = err + @_applicationCache = null + @trigger(@) \ No newline at end of file diff --git a/src/components/resizable-region.cjsx b/src/components/resizable-region.cjsx index 38b388e24..f296b98e2 100644 --- a/src/components/resizable-region.cjsx +++ b/src/components/resizable-region.cjsx @@ -1,6 +1,8 @@ React = require 'react' _ = require 'underscore-plus' -{Actions,ComponentRegistry} = require "inbox-exports" +{Actions, + ComponentRegistry, + PriorityUICoordinator} = require "inbox-exports" ResizableHandle = Top: @@ -96,6 +98,10 @@ ResizableRegion = React.createClass @setState(height: nextProps.initialHeight) if nextProps.handle.axis is 'horizontal' and nextProps.initialWidth != @props.initialWidth @setState(width: nextProps.initialWidth) + + componentWillUnmount: -> + PriorityUICoordinator.endPriorityTask(@_taskId) if @_taskId + @_taskId = null _mouseDown: (event) -> return if event.button != 0 @@ -106,6 +112,9 @@ ResizableRegion = React.createClass event.stopPropagation() event.preventDefault() + PriorityUICoordinator.endPriorityTask(@_taskId) if @_taskId + @_taskId = PriorityUICoordinator.beginPriorityTask() + _mouseUp: (event) -> return if event.button != 0 @setState @@ -114,6 +123,9 @@ ResizableRegion = React.createClass event.stopPropagation() event.preventDefault() + PriorityUICoordinator.endPriorityTask(@_taskId) + @_taskId = null + _mouseMove: (event) -> return if !@state.dragging @setState @props.handle.transform(@state, @props, event) diff --git a/src/components/timeout-transition-group.cjsx b/src/components/timeout-transition-group.cjsx index 89f76d8ea..dd86e11b0 100644 --- a/src/components/timeout-transition-group.cjsx +++ b/src/components/timeout-transition-group.cjsx @@ -23,13 +23,13 @@ ### React = require('react/addons') +PriorityUICoordinator = require('../priority-ui-coordinator') ReactTransitionGroup = React.addons.TransitionGroup TICK = 17 endEvents = ['webkitTransitionEnd', 'webkitAnimationEnd'] -animationSupported = -> - endEvents.length != 0 +animationSupported = -> true ###* # Functions for element class management to replace dependency on jQuery @@ -63,12 +63,31 @@ TimeoutTransitionGroupChild = React.createClass( className = @props.name + '-' + animationType activeClassName = className + '-active' - endListener = -> - removeClass node, className - removeClass node, activeClassName + # If you animate back and forth fast enough, you can call `transition` + # before a previous transition has finished. Make sure we cancel the + # old timeout. + if @animationTimeout + clearTimeout(@animationTimeout) + @animationTimeout = null + + if @animationTaskId + PriorityUICoordinator.endPriorityTask(@animationTaskId) + @animationTaskId = null + + # Block database responses, JSON parsing while we are in flight + @animationTaskId = PriorityUICoordinator.beginPriorityTask() + + endListener = => + removeClass(node, className) + removeClass(node, activeClassName) # Usually this optional callback is used for informing an owner of # a leave animation and telling it to remove the child. finishCallback and finishCallback() + + if @animationTaskId + PriorityUICoordinator.endPriorityTask(@animationTaskId) + @animationTaskId = null + @animationTimeout = null return if !animationSupported() @@ -78,7 +97,9 @@ TimeoutTransitionGroupChild = React.createClass( @animationTimeout = setTimeout(endListener, @props.enterTimeout) else if animationType == 'leave' @animationTimeout = setTimeout(endListener, @props.leaveTimeout) - addClass node, className + + addClass(node, className) + # Need to do this to actually trigger a transition. @queueClass activeClassName return @@ -88,11 +109,11 @@ TimeoutTransitionGroupChild = React.createClass( if !@timeout @timeout = setTimeout(@flushClassNameQueue, TICK) return - + flushClassNameQueue: -> if @isMounted() @classNameQueue.forEach ((name) -> - addClass @getDOMNode(), name + addClass(@getDOMNode(), name) return ).bind(this) @classNameQueue.length = 0 @@ -101,13 +122,19 @@ TimeoutTransitionGroupChild = React.createClass( componentWillMount: -> @classNameQueue = [] + @animationTimeout = null + @animationTaskId = null return componentWillUnmount: -> if @timeout - clearTimeout @timeout + clearTimeout(@timeout) if @animationTimeout - clearTimeout @animationTimeout + clearTimeout(@animationTimeout) + @animationTimeout = null + if @animationTaskId + PriorityUICoordinator.endPriorityTask(@animationTaskId) + @animationTaskId = null return componentWillEnter: (done) -> diff --git a/src/flux/edgehill-api.coffee b/src/flux/edgehill-api.coffee index 8ea5be23f..c29c40686 100644 --- a/src/flux/edgehill-api.coffee +++ b/src/flux/edgehill-api.coffee @@ -2,6 +2,7 @@ nodeRequest = require 'request' Actions = require './actions' {APIError} = require './errors' DatabaseStore = require './stores/database-store' +PriorityUICoordinator = require '../priority-ui-coordinator' {modelFromJSON} = require './models/utils' async = require 'async' @@ -38,11 +39,12 @@ class EdgehillAPI options.error ?= @_defaultErrorCallback nodeRequest options, (error, response, body) -> - Actions.didMakeAPIRequest({request: options, response: response}) - if error? or response.statusCode > 299 - options.error(new APIError({error:error, response:response, body:body, requestOptions: options})) - else - options.success(body) if options.success + PriorityUICoordinator.settle.then -> + Actions.didMakeAPIRequest({request: options, response: response}) + if error? or response.statusCode > 299 + options.error(new APIError({error:error, response:response, body:body, requestOptions: options})) + else + options.success(body) if options.success urlForConnecting: (provider, email_address = '') -> auth = @getCredentials() diff --git a/src/flux/inbox-api.coffee b/src/flux/inbox-api.coffee index 19e544fa7..c4e755e20 100644 --- a/src/flux/inbox-api.coffee +++ b/src/flux/inbox-api.coffee @@ -2,6 +2,7 @@ _ = require 'underscore-plus' request = require 'request' Actions = require './actions' {APIError} = require './errors' +PriorityUICoordinator = require '../priority-ui-coordinator' DatabaseStore = require './stores/database-store' NamespaceStore = require './stores/namespace-store' InboxLongConnection = require './inbox-long-connection' @@ -84,7 +85,8 @@ class InboxAPI Actions.longPollOffline() connection.onDeltas (deltas) => - @_handleDeltas(deltas) + PriorityUICoordinator.settle.then => + @_handleDeltas(deltas) connection.start() connection @@ -112,17 +114,18 @@ class InboxAPI options.error ?= @_defaultErrorCallback request options, (error, response, body) => - Actions.didMakeAPIRequest({request: options, response: response}) - if error? or response.statusCode > 299 - options.error(new APIError({error:error, response:response, body:body})) - else - if _.isString body - try - body = JSON.parse(body) - catch error - options.error(new APIError({error:error, response:response, body:body})) - @_handleModelResponse(body) if options.returnsModel - options.success(body) if options.success + PriorityUICoordinator.settle.then => + Actions.didMakeAPIRequest({request: options, response: response}) + if error? or response.statusCode > 299 + options.error(new APIError({error:error, response:response, body:body})) + else + if _.isString body + try + body = JSON.parse(body) + catch error + options.error(new APIError({error:error, response:response, body:body})) + @_handleModelResponse(body) if options.returnsModel + options.success(body) if options.success _handleDeltas: (deltas) -> Actions.longPollReceivedRawDeltas(deltas) diff --git a/src/flux/models/query.coffee b/src/flux/models/query.coffee index f283f2b30..56fa63b5b 100644 --- a/src/flux/models/query.coffee +++ b/src/flux/models/query.coffee @@ -8,6 +8,7 @@ class ModelQuery @_matchers = [] @_orders = [] @_singular = false + @_evaluateImmediately = false @_includeJoinedData = [] @_count = false @ @@ -61,6 +62,9 @@ class ModelQuery count: -> @_count = true @ + + evaluateImmediately: -> + @_evaluateImmediately = true # Query Execution @@ -107,6 +111,9 @@ class ModelQuery limit += " OFFSET #{@_range.offset}" "SELECT #{result} FROM `#{@_klass.name}` #{@_whereClause()} #{order} #{limit}" + executeOptions: -> + evaluateImmediately: @_evaluateImmediately + _whereClause: -> joins = [] @_matchers.forEach (c) => diff --git a/src/flux/stores/database-store.coffee b/src/flux/stores/database-store.coffee index b5a458586..ac2af3e2b 100644 --- a/src/flux/stores/database-store.coffee +++ b/src/flux/stores/database-store.coffee @@ -6,6 +6,7 @@ Actions = require '../actions' Model = require '../models/model' LocalLink = require '../models/local-link' ModelQuery = require '../models/query' +PriorityUICoordinator = require '../../priority-ui-coordinator' {AttributeCollection, AttributeJoinedData} = require '../attributes' {modelFromJSON, modelClassMap, tableNameForJoin, generateTempId, isTempId} = require '../models/utils' fs = require 'fs-plus' @@ -22,20 +23,28 @@ verboseFilter = (query) -> class DatabaseProxy constructor: (@databasePath) -> @windowId = remote.getCurrentWindow().id - @queryCallbacks = {} + @queryRecords = {} @queryId = 0 ipc.on 'database-result', ({queryKey, err, result}) => - @queryCallbacks[queryKey](err, result) if @queryCallbacks[queryKey] + {callback, options} = @queryRecords[queryKey] console.timeStamp("DB END #{queryKey}. #{result?.length} chars") - delete @queryCallbacks[queryKey] + + waits = Promise.resolve() + waits = PriorityUICoordinator.settle unless options.evaluateImmediately + waits.then => + callback(err, result) if callback + delete @queryRecords[queryKey] @ - query: (query, values, callback) -> + query: (query, values, callback, options) -> @queryId += 1 queryKey = "#{@windowId}-#{@queryId}" - @queryCallbacks[queryKey] = callback if callback + @queryRecords[queryKey] = { + callback: callback, + options: options + } console.timeStamp("DB SEND #{queryKey}: #{query}") console.log(query) if verboseFilter(query) ipc.send('database-query', {@databasePath, queryKey, query, values}) @@ -47,7 +56,7 @@ class DatabasePromiseTransaction constructor: (@_db, @_resolve, @_reject) -> @_running = 0 - execute: (query, values, querySuccess, queryFailure) -> + execute: (query, values, querySuccess, queryFailure, options = {}) -> # Wrap any user-provided success callback in one that checks query time callback = (err, result) => if err @@ -66,7 +75,7 @@ class DatabasePromiseTransaction @_resolve(result) @_running += 1 - @_db.query(query, values || [], callback) + @_db.query(query, values || [], callback, options) executeInSeries: (queries) -> async.eachSeries queries @@ -354,7 +363,7 @@ DatabaseStore = Reflux.createStore run: (modelQuery) -> @inTransaction {readonly: true}, (tx) -> - tx.execute modelQuery.sql(), [] + tx.execute(modelQuery.sql(), [], null, null, modelQuery.executeOptions()) .then (result) -> Promise.resolve(modelQuery.formatResult(result)) diff --git a/src/flux/stores/message-store.coffee b/src/flux/stores/message-store.coffee index d45e22ded..01bf95db9 100644 --- a/src/flux/stores/message-store.coffee +++ b/src/flux/stores/message-store.coffee @@ -84,6 +84,7 @@ MessageStore = Reflux.createStore query = DatabaseStore.findAll(Message, threadId: loadedThreadId) query.include(Message.attributes.body) + query.evaluateImmediately() query.then (items) => localIds = {} async.each items, (item, callback) -> diff --git a/src/priority-ui-coordinator.coffee b/src/priority-ui-coordinator.coffee new file mode 100644 index 000000000..877afa522 --- /dev/null +++ b/src/priority-ui-coordinator.coffee @@ -0,0 +1,43 @@ +{generateTempId} = require './flux/models/utils' + +# A small object that keeps track of the current animation state of the +# application. You can use it to defer work until animations have finished. +# Integrated with our fork of TimeoutTransitionGroup +# +# PriorityUICoordinator.settle.then -> +# # Do something expensive +# +class PriorityUICoordinator + constructor: -> + @tasks = {} + @settle = Promise.resolve() + setInterval(( => @detectOrphanedTasks()), 1000) + + beginPriorityTask: -> + if Object.keys(@tasks).length is 0 + @settle = new Promise (resolve, reject) => + @settlePromiseResolve = resolve + + id = generateTempId() + @tasks[id] = Date.now() + id + + endPriorityTask: (id) -> + throw new Error("You must provide a task id to endPriorityTask") unless id + delete @tasks[id] + if Object.keys(@tasks).length is 0 + @settlePromiseResolve() if @settlePromiseResolve + @settlePromiseResolve = null + + detectOrphanedTasks: -> + now = Date.now() + threshold = 15000 # milliseconds + for id, timestamp of @tasks + if now - timestamp > threshold + console.log("PriorityUICoordinator detected oprhaned priority task lasting #{threshold}ms. Ending.") + @endPriorityTask(id) + + busy: -> + Object.keys(@tasks).length > 0 + +module.exports = new PriorityUICoordinator() \ No newline at end of file