From 7968f26282d6386c04e922c1c71a53dccaf0167a Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Mon, 3 Aug 2015 18:39:11 -0700 Subject: [PATCH] fix(draft-scroll): Ignore composer scroll requests while moving to new draft --- .../message-list/lib/message-list.cjsx | 9 +++++++++ src/components/scroll-region.cjsx | 20 +++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index 8a7171757..4239394d5 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -93,6 +93,7 @@ class MessageList extends React.Component constructor: (@props) -> @state = @_getStateFromStores() @state.minified = true + @_draftScrollInProgress = false @MINIFY_THRESHOLD = 3 componentDidMount: => @@ -131,10 +132,17 @@ class MessageList extends React.Component @refs["message-container-#{id}"] _focusDraft: (draftElement) => + # Note: We don't want the contenteditable view competing for scroll offset, + # so we block incoming childScrollRequests while we scroll to the new draft. + @_draftScrollInProgress = true draftElement.focus() @refs.messageWrap.scrollTo(draftElement, { position: ScrollRegion.ScrollPosition.Bottom, settle: true + position: ScrollRegion.ScrollPosition.Top, + settle: true, + done: => + @_draftScrollInProgress = false }) _createReplyOrUpdateExistingDraft: (type) => @@ -369,6 +377,7 @@ class MessageList extends React.Component # If messageId and location are defined, that means we want to scroll # smoothly to the top of a particular message. _onChildScrollRequest: ({messageId, rect}={}) => + return if @_draftScrollInProgress if messageId @refs.messageWrap.scrollTo(@_getMessageContainer(messageId), { position: ScrollRegion.ScrollPosition.Visible diff --git a/src/components/scroll-region.cjsx b/src/components/scroll-region.cjsx index c82504f32..52a0731fd 100644 --- a/src/components/scroll-region.cjsx +++ b/src/components/scroll-region.cjsx @@ -194,26 +194,26 @@ class ScrollRegion extends React.Component # Public: Scroll to the DOM Node provided. # - scrollTo: (node, {position, settle} = {}) => + scrollTo: (node, {position, settle, done} = {}) => if node instanceof React.Component node = React.findDOMNode(node) unless node instanceof Node throw new Error("ScrollRegion.scrollTo: requires a DOM node or React element. Maybe you meant scrollToRect?") - @_scroll {position, settle}, => + @_scroll {position, settle, done}, => node.getBoundingClientRect() # Public: Scroll to the client rectangle provided. Note: This method expects # a ClientRect or similar object with top, left, width, height relative to the # window, not the scroll region. This is designed to make it easy to use with # node.getBoundingClientRect() - scrollToRect: (rect, {position, settle} = {}) -> + scrollToRect: (rect, {position, settle, done} = {}) -> if rect instanceof Node throw new Error("ScrollRegion.scrollToRect: requires a rect. Maybe you meant scrollTo?") if not rect.top or not rect.height throw new Error("ScrollRegion.scrollToRect: requires a rect with `top` and `height` attributes.") - @_scroll {position, settle}, => rect + @_scroll {position, settle, done}, => rect - _scroll: ({position, settle}, clientRectProviderCallback) -> + _scroll: ({position, settle, done}, clientRectProviderCallback) -> contentNode = React.findDOMNode(@refs.content) position ?= ScrollRegion.ScrollPosition.Visible @@ -227,7 +227,7 @@ class ScrollRegion extends React.Component settleFn => # If another scroll call has been made since ours, don't do anything. - return unless @_scrollToTaskId is taskId + return done?(false) unless @_scrollToTaskId is taskId contentClientRect = contentNode.getBoundingClientRect() rect = _.clone(clientRectProviderCallback()) @@ -260,20 +260,18 @@ class ScrollRegion extends React.Component else if distanceAboveTop >= 0 @scrollTop -= distanceAboveTop + done?(true) + _settleHeight: (callback) => contentNode = React.findDOMNode(@refs.content) lastContentHeight = -1 - stableCount = 0 scrollIfSettled = => return unless @_mounted contentRect = contentNode.getBoundingClientRect() if contentRect.height isnt lastContentHeight lastContentHeight = contentRect.height - stableCount = 0 else - stableCount += 1 - if stableCount is 5 - return callback() + return callback() window.requestAnimationFrame(scrollIfSettled) scrollIfSettled()