From 9b2f830348ba06369e4a4f065885008606a95e52 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Mon, 3 Aug 2015 17:05:31 -0700 Subject: [PATCH] fix(*): Minor performance tweaks and fixes to category picker Summary: fix(undo-redo): UndoRedoComponent does not take props fix(category-picker): - Use Actions.queueTask like the rest of the app so UndoRedoStore can see it. Can change this in the future but it's currently the only place in the app we directly queue tasks. - Stop subscribing to the FocusedContentStore / FocusedCategoryStore (which are not used in setState?) since we receive threads as props - Rename categoryDatum to item because it's not a category. (Was super confused that categories were becoming JSON in `_extendCategoryWithDisplayData`) Give item a category property so that tasks can specify items and not IDs (allows for better descriptions like "Moved one thread to Archive" Add simple shouldComponentUpdate to retina-img Test Plan: Run tests Reviewers: evan Reviewed By: evan Differential Revision: https://phab.nylas.com/D1832 --- .../category-picker/lib/category-picker.cjsx | 75 ++++++++++--------- .../composer/lib/composer-view.cjsx | 1 - .../composer/stylesheets/composer.less | 8 +- .../message-list/lib/message-list.cjsx | 16 ++-- .../stylesheets/message-list.less | 2 +- .../undo-redo/lib/undo-redo-component.cjsx | 4 - src/components/retina-img.cjsx | 7 +- static/components/tokenizing-text-field.less | 4 +- 8 files changed, 56 insertions(+), 61 deletions(-) diff --git a/internal_packages/category-picker/lib/category-picker.cjsx b/internal_packages/category-picker/lib/category-picker.cjsx index 7a247d1f5..9303647d8 100644 --- a/internal_packages/category-picker/lib/category-picker.cjsx +++ b/internal_packages/category-picker/lib/category-picker.cjsx @@ -10,7 +10,6 @@ React = require 'react' WorkspaceStore, ChangeLabelsTask, ChangeFolderTask, - FocusedContentStore, FocusedCategoryStore} = require 'nylas-exports' {Menu, @@ -33,8 +32,6 @@ class CategoryPicker extends React.Component @unsubscribers = [] @unsubscribers.push CategoryStore.listen @_onStoreChanged @unsubscribers.push NamespaceStore.listen @_onStoreChanged - @unsubscribers.push FocusedContentStore.listen @_onStoreChanged - @unsubscribers.push FocusedCategoryStore.listen @_onStoreChanged @_commandUnsubscriber = atom.commands.add 'body', "application:change-category": @_onOpenCategoryPopover @@ -94,7 +91,7 @@ class CategoryPicker extends React.Component headerComponents={headerComponents} footerComponents={[]} items={@state.categoryData} - itemKey={ (categoryDatum) -> categoryDatum.id } + itemKey={ (item) -> item.id } itemContent={@_renderItemContent} onSelect={@_onSelectCategory} defaultSelectedIndex={if @state.searchValue is "" then -1 else 0} @@ -107,55 +104,55 @@ class CategoryPicker extends React.Component @refs.popover.open() return - _renderItemContent: (categoryDatum) => - if categoryDatum.divider - return + _renderItemContent: (item) => + if item.divider + return if @_namespace?.usesLabels() - icon = @_renderCheckbox(categoryDatum) + icon = @_renderCheckbox(item) else if @_namespace?.usesFolders() - icon = @_renderFolderIcon(categoryDatum) + icon = @_renderFolderIcon(item) else return
{icon}
- {@_renderBoldedSearchResults(categoryDatum)} + {@_renderBoldedSearchResults(item)}
- _renderCheckbox: (categoryDatum) -> + _renderCheckbox: (item) -> styles = {} - styles.backgroundColor = categoryDatum.backgroundColor + styles.backgroundColor = item.backgroundColor - if categoryDatum.usage is 0 + if item.usage is 0 checkStatus = - else if categoryDatum.usage < categoryDatum.numThreads + else if item.usage < item.numThreads checkStatus = @_onSelectCategory(categoryDatum)}/> + onClick={=> @_onSelectCategory(item)}/> else checkStatus = @_onSelectCategory(categoryDatum)}/> + onClick={=> @_onSelectCategory(item)}/>
@_onSelectCategory(categoryDatum)}/> + onClick={=> @_onSelectCategory(item)}/> {checkStatus}
- _renderFolderIcon: (categoryDatum) -> - + _renderFolderIcon: (item) -> + - _renderBoldedSearchResults: (categoryDatum) -> - name = categoryDatum.display_name + _renderBoldedSearchResults: (item) -> + name = item.display_name searchTerm = (@state.searchValue ? "").trim() return name if searchTerm.length is 0 @@ -173,33 +170,35 @@ class CategoryPicker extends React.Component else return part return {parts} - _onSelectCategory: (categoryDatum) => + _onSelectCategory: (item) => return unless @_threads().length > 0 return unless @_namespace @refs.menu.setSelectedItem(null) if @_namespace.usesLabels() - if categoryDatum.usage > 0 + if item.usage > 0 task = new ChangeLabelsTask - labelsToRemove: [categoryDatum.id] + labelsToRemove: [item.category] threadIds: @_threadIds() else task = new ChangeLabelsTask - labelsToAdd: [categoryDatum.id] + labelsToAdd: [item.category] threadIds: @_threadIds() + Actions.queueTask(task) + else if @_namespace.usesFolders() task = new ChangeFolderTask - folderOrId: categoryDatum.id + folderOrId: item.category threadIds: @_threadIds() if @props.thread Actions.moveThread(@props.thread, task) else if @props.items Actions.moveThreads(@_threads(), task) - else throw new Error("Invalid organizationUnit") + else + throw new Error("Invalid organizationUnit") @refs.popover.close() - TaskQueue.enqueue(task) _onStoreChanged: => @setState @_recalculateState(@props) @@ -219,6 +218,7 @@ class CategoryPicker extends React.Component numThreads = @_threads(props).length if numThreads is 0 return {categoryData: [], searchValue} + @_namespace = NamespaceStore.current() return unless @_namespace @@ -235,7 +235,7 @@ class CategoryPicker extends React.Component categoryData = _.chain(categories) .filter(_.partial(@_isUserFacing, allInInbox)) .filter(_.partial(@_isInSearch, searchValue)) - .map(_.partial(@_extendCategoryWithDisplayData, displayData)) + .map(_.partial(@_itemForCategory, displayData)) .value() return {categoryData, searchValue} @@ -267,14 +267,15 @@ class CategoryPicker extends React.Component inbox = CategoryStore.getStandardCategory("inbox") return usageCount[inbox.id] is numThreads - _extendCategoryWithDisplayData: ({usageCount, numThreads}, category) -> + _itemForCategory: ({usageCount, numThreads}, category) -> return category if category.divider - cat = category.toJSON() - usage = usageCount[cat.id] ? 0 - cat.backgroundColor = LabelColorizer.backgroundColorDark(category) - cat.usage = usage - cat.numThreads = numThreads - return cat + + item = category.toJSON() + item.category = category + item.backgroundColor = LabelColorizer.backgroundColorDark(category) + item.usage = usageCount[category.id] ? 0 + item.numThreads = numThreads + item _threadCategories: (thread) => if @_namespace.usesLabels() @@ -283,7 +284,7 @@ class CategoryPicker extends React.Component return (thread.folders ? []) else throw new Error("Invalid organizationUnit") - _threads: (props=@props) => + _threads: (props = @props) => if props.items then return (props.items ? []) else if props.thread then return [props.thread] else return [] diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index c7776c962..f8de4877e 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -261,7 +261,6 @@ class ComposerView extends React.Component tabIndex="108" placeholder="Subject" disabled={not @state.showsubject} - className="compose-field compose-subject" value={@state.subject} onChange={@_onChangeSubject}/> diff --git a/internal_packages/composer/stylesheets/composer.less b/internal_packages/composer/stylesheets/composer.less index d137bf20f..6fc4547da 100644 --- a/internal_packages/composer/stylesheets/composer.less +++ b/internal_packages/composer/stylesheets/composer.less @@ -134,7 +134,6 @@ .compose-subject-wrap { position: relative; z-index: 2; - padding: 11px @spacing-standard 11px 0; margin: 0 23px; border-bottom: 1px solid @border-color-divider; flex-shrink:0; @@ -146,16 +145,15 @@ display: block; } - input.compose-field { + input { display: inline-block; - width: calc(~"100% - 61px"); - padding: 0; + padding: 13px 0 9px 0; margin: 0 0 0 5px; min-width: 5em; background-color: transparent; border: none; } - input.compose-field.compose-subject { + input { &::-webkit-input-placeholder { color: @text-color-very-subtle; } diff --git a/internal_packages/message-list/lib/message-list.cjsx b/internal_packages/message-list/lib/message-list.cjsx index 41102fb69..723425e9b 100755 --- a/internal_packages/message-list/lib/message-list.cjsx +++ b/internal_packages/message-list/lib/message-list.cjsx @@ -245,15 +245,12 @@ class MessageList extends React.Component @_onRemoveLabel(label) }/> _renderReplyArea: => - if @_hasReplyArea() -
-
- - Write a reply… -
+
+
+ + Write a reply…
- else -
+
_hasReplyArea: => not _.last(@state.messages)?.draft @@ -308,7 +305,8 @@ class MessageList extends React.Component collapsed={collapsed} isLastMsg={(messages.length - 1 is idx)} /> - components.push @_renderReplyArea() + if @_hasReplyArea() + components.push @_renderReplyArea() return components diff --git a/internal_packages/message-list/stylesheets/message-list.less b/internal_packages/message-list/stylesheets/message-list.less index 0ba9ef514..ecf949c91 100644 --- a/internal_packages/message-list/stylesheets/message-list.less +++ b/internal_packages/message-list/stylesheets/message-list.less @@ -129,7 +129,7 @@ margin: 0 auto; padding: @message-spacing 0; &:last-child { - padding-bottom: @message-spacing * 2; + padding-bottom: @spacing-double; } .message-item-white-wrap { background: @background-primary; diff --git a/internal_packages/undo-redo/lib/undo-redo-component.cjsx b/internal_packages/undo-redo/lib/undo-redo-component.cjsx index 4a0af6970..09d2635fa 100644 --- a/internal_packages/undo-redo/lib/undo-redo-component.cjsx +++ b/internal_packages/undo-redo/lib/undo-redo-component.cjsx @@ -12,10 +12,6 @@ NamespaceStore} = require 'nylas-exports' class UndoRedoComponent extends React.Component @displayName: 'UndoRedoComponent' - @propTypes: - task: React.PropTypes.object.isRequired - show: React.PropTypes.bool - constructor: (@props) -> @state = @_getStateFromStores() @_timeout = null diff --git a/src/components/retina-img.cjsx b/src/components/retina-img.cjsx index 8edd0d5df..5fc6af406 100644 --- a/src/components/retina-img.cjsx +++ b/src/components/retina-img.cjsx @@ -98,7 +98,10 @@ class RetinaImg extends React.Component active: React.PropTypes.bool resourcePath: React.PropTypes.string - render: -> + shouldComponentUpdate: (nextProps) => + not _.isEqual(@props, nextProps) + + render: => path = @props.url ? @_pathFor(@props.name) ? @_pathFor(@props.fallback) ? '' pathIsRetina = path.indexOf('@2x') > 0 className = @props.className ? '' @@ -125,7 +128,7 @@ class RetinaImg extends React.Component otherProps = _.omit(@props, _.keys(@constructor.propTypes)) - _pathFor: (name) -> + _pathFor: (name) => return null unless name and _.isString(name) [basename, ext] = name.split('.') diff --git a/static/components/tokenizing-text-field.less b/static/components/tokenizing-text-field.less index e02c81c26..2a71a860b 100644 --- a/static/components/tokenizing-text-field.less +++ b/static/components/tokenizing-text-field.less @@ -17,7 +17,7 @@ margin: 0; padding: 0; border-bottom: 1px solid @border-color-divider; - min-height: 30px; + min-height: 42px; position: relative; .content-container { @@ -134,7 +134,7 @@ .tokenizing-field-input { position: relative; - padding-left: 2.1em; + padding-left: 2.8em; &:hover { cursor: text;