From 0c2bcd3fba9d784f27adc6f172dd46e1b6a700de Mon Sep 17 00:00:00 2001 From: Evan Morikawa Date: Thu, 12 Mar 2015 21:47:01 -0400 Subject: [PATCH] fix(toolbar): slash focuses search & hidden buttons don't act Test Plan: edgehill --test Reviewers: bengotow Reviewed By: bengotow Differential Revision: https://review.inboxapp.com/D1289 --- .../message-list/lib/message-toolbar-items.cjsx | 6 +++++- internal_packages/search-bar/lib/search-bar.cjsx | 9 +++++++++ internal_packages/tooltip/lib/tooltip.cjsx | 13 ++----------- keymaps/base.cson | 2 ++ src/flux/models/utils.coffee | 15 +++++++++++++++ 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/internal_packages/message-list/lib/message-toolbar-items.cjsx b/internal_packages/message-list/lib/message-toolbar-items.cjsx index bde69810c..58ab6e654 100644 --- a/internal_packages/message-list/lib/message-toolbar-items.cjsx +++ b/internal_packages/message-list/lib/message-toolbar-items.cjsx @@ -1,5 +1,5 @@ React = require 'react' -{Actions, ThreadStore} = require 'inbox-exports' +{Actions, ThreadStore, Utils} = require 'inbox-exports' {RetinaImg} = require 'ui-components' # Note: These always have a thread, but only sometimes get a @@ -14,6 +14,7 @@ ReplyButton = React.createClass _onReply: (e) -> + return unless Utils.nodeIsVisible(e.currentTarget) Actions.composeReply(threadId: ThreadStore.selectedId()) e.stopPropagation() @@ -26,6 +27,7 @@ ReplyAllButton = React.createClass _onReplyAll: (e) -> + return unless Utils.nodeIsVisible(e.currentTarget) Actions.composeReplyAll(threadId: ThreadStore.selectedId()) e.stopPropagation() @@ -38,6 +40,7 @@ ForwardButton = React.createClass _onForward: (e) -> + return unless Utils.nodeIsVisible(e.currentTarget) Actions.composeForward(threadId: ThreadStore.selectedId()) e.stopPropagation() @@ -50,6 +53,7 @@ ArchiveButton = React.createClass _onArchive: (e) -> + return unless Utils.nodeIsVisible(e.currentTarget) # Calling archive() sends an Actions.queueTask with an archive task # TODO Turn into an Action ThreadStore.selectedThread().archive() diff --git a/internal_packages/search-bar/lib/search-bar.cjsx b/internal_packages/search-bar/lib/search-bar.cjsx index 657b0c253..6e67345ab 100644 --- a/internal_packages/search-bar/lib/search-bar.cjsx +++ b/internal_packages/search-bar/lib/search-bar.cjsx @@ -16,12 +16,16 @@ SearchBar = React.createClass componentDidMount: -> @unsubscribe = SearchSuggestionStore.listen @_onStoreChange + @body_unsubscriber = atom.commands.add 'body', { + 'application:focus-search': @_onFocusSearch + } # It's important that every React class explicitly stops listening to # atom events before it unmounts. Thank you event-kit # This can be fixed via a Reflux mixin componentWillUnmount: -> @unsubscribe() + @body_unsubscriber.dispose() render: -> inputValue = @_queryToString(@state.query) @@ -31,6 +35,7 @@ SearchBar = React.createClass headerComponents = [ + _onFocusSearch: -> + return unless @isMounted() + @refs.searchInput.getDOMNode().focus() + _containerClasses: -> React.addons.classSet 'focused': @state.focused diff --git a/internal_packages/tooltip/lib/tooltip.cjsx b/internal_packages/tooltip/lib/tooltip.cjsx index 7663ed99b..b2afd306a 100644 --- a/internal_packages/tooltip/lib/tooltip.cjsx +++ b/internal_packages/tooltip/lib/tooltip.cjsx @@ -1,5 +1,6 @@ _ = require 'underscore-plus' React = require 'react/addons' +{Utils} = require 'inbox-exports' ### The Tooltip component displays a consistent hovering tooltip for use when @@ -47,23 +48,13 @@ Tooltip = React.createClass # listeners. onMouseOver: (e) -> target = @_elementWithTooltip(e.target) - if target and @_targetIsVisible(target) then @_onTooltipEnter(target) + if target and Utils.nodeIsVisible(target) then @_onTooltipEnter(target) else if @state.display then @_hideTooltip() onMouseOut: (e) -> if @_elementWithTooltip(e.fromElement) and not @_elementWithTooltip(e.toElement) @_onTooltipLeave() - _targetIsVisible: (target) -> - while target - style = window.getComputedStyle(target) - target = target.parentNode - continue unless style? - # NOTE: opacity must be soft == - if style.opacity is 0 or style.opacity is "0" or style.visibility is "hidden" or style.display is "none" - return false - return true - _elementWithTooltip: (target) -> while target break if target?.dataset?.tooltip? diff --git a/keymaps/base.cson b/keymaps/base.cson index 7d3758fe3..2ebcc24b9 100644 --- a/keymaps/base.cson +++ b/keymaps/base.cson @@ -16,6 +16,8 @@ 'delete' : 'application:remove-item' # Mac mail 'backspace': 'application:remove-item' # Outlook + '/' : 'application:focus-search' # Gmail + 'r' : 'application:reply' # Gmail 'a' : 'application:reply-all' # Gmail 'f' : 'application:forward' # Gmail diff --git a/src/flux/models/utils.coffee b/src/flux/models/utils.coffee index b08d32b63..21435a627 100644 --- a/src/flux/models/utils.coffee +++ b/src/flux/models/utils.coffee @@ -149,4 +149,19 @@ Utils = # Return remaining compacted email body lines.join('\n') + # Checks to see if a particular node is visible and any of its parents + # are visible. + # + # WARNING. This is a fairly expensive operation and should be used + # sparingly. + nodeIsVisible: (node) -> + while node + style = window.getComputedStyle(node) + node = node.parentNode + continue unless style? + # NOTE: opacity must be soft == + if style.opacity is 0 or style.opacity is "0" or style.visibility is "hidden" or style.display is "none" + return false + return true + module.exports = Utils