Group buttons in the thread toolbar for nicer layout

app/internal_packages/category-picker/lib/label-picker.jsx

@@ -1,83 +0,0 @@
-const { Actions, AccountStore, React, PropTypes, WorkspaceStore } = require('mailspring-exports');
-const { RetinaImg, KeyCommandsRegion } = require('mailspring-component-kit');
-
-const LabelPickerPopover = require('./label-picker-popover').default;
-
-// This changes the category on one or more threads.
-class LabelPicker extends React.Component {
-  static displayName = 'LabelPicker';
-  static containerRequired = false;
-  static propTypes = { items: PropTypes.array };
-  static contextTypes = { sheetDepth: PropTypes.number };
-
-  constructor(props) {
-    super(props);
-    this._account = AccountStore.accountForItems(this.props.items);
-  }
-
-  // If the threads we're picking categories for change, (like when they
-  // get their categories updated), we expect our parents to pass us new
-  // props. We don't listen to the DatabaseStore ourselves.
-  componentWillReceiveProps(nextProps) {
-    return (this._account = AccountStore.accountForItems(nextProps.items));
-  }
-
-  _keymapHandlers() {
-    return { 'core:change-labels': this._onOpenCategoryPopover };
-  }
-
-  _onOpenCategoryPopover = () => {
-    if (!(this.props.items.length > 0)) {
-      return;
-    }
-    if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) {
-      return;
-    }
-    const buttonRect = this._buttonEl.getBoundingClientRect();
-    Actions.openPopover(<LabelPickerPopover threads={this.props.items} account={this._account} />, {
-      originRect: buttonRect,
-      direction: 'down',
-    });
-  };
-
-  render() {
-    if (!this._account) {
-      return <span />;
-    }
-    if (!this._account.usesLabels()) {
-      return <span />;
-    }
-    const btnClasses = 'btn btn-toolbar btn-category-picker';
-
-    return (
-      <KeyCommandsRegion
-        style={{ order: -103 }}
-        globalHandlers={this._keymapHandlers()}
-        globalMenuItems={[
-          {
-            label: 'Thread',
-            submenu: [
-              {
-                label: 'Apply Labels...',
-                command: 'core:change-labels',
-                position: 'endof=thread-actions',
-              },
-            ],
-          },
-        ]}
-      >
-        <button
-          tabIndex={-1}
-          ref={el => (this._buttonEl = el)}
-          title={'Apply Labels'}
-          onClick={this._onOpenCategoryPopover}
-          className={btnClasses}
-        >
-          <RetinaImg name={'toolbar-tag.png'} mode={RetinaImg.Mode.ContentIsMask} />
-        </button>
-      </KeyCommandsRegion>
-    );
-  }
-}
-
-module.exports = LabelPicker;

app/internal_packages/category-picker/lib/main.jsx

@@ -1,17 +1,12 @@
-const MovePicker = require('./move-picker');
+const ToolbarCategoryPicker = require('./toolbar-category-picker');
-const LabelPicker = require('./label-picker');
-
 const { ComponentRegistry } = require('mailspring-exports');
 
 module.exports = {
-  activate(state = {}) {
+  activate() {
-    this.state = state;
+    ComponentRegistry.register(ToolbarCategoryPicker, { role: 'ThreadActionsToolbarButton' });
-    ComponentRegistry.register(MovePicker, { role: 'ThreadActionsToolbarButton' });
-    ComponentRegistry.register(LabelPicker, { role: 'ThreadActionsToolbarButton' });
   },
 
   deactivate() {
-    ComponentRegistry.unregister(MovePicker);
+    ComponentRegistry.unregister(ToolbarCategoryPicker);
-    ComponentRegistry.unregister(LabelPicker);
   },
 };

app/internal_packages/category-picker/lib/move-picker.jsx

@@ -1,82 +0,0 @@
-const { Actions, React, PropTypes, AccountStore, WorkspaceStore } = require('mailspring-exports');
-const { RetinaImg, KeyCommandsRegion } = require('mailspring-component-kit');
-
-const MovePickerPopover = require('./move-picker-popover').default;
-
-// This sets the folder / label on one or more threads.
-class MovePicker extends React.Component {
-  static displayName = 'MovePicker';
-  static containerRequired = false;
-
-  static propTypes = { items: PropTypes.array };
-  static contextTypes = { sheetDepth: PropTypes.number };
-
-  constructor(props) {
-    super(props);
-
-    this._account = AccountStore.accountForItems(this.props.items);
-  }
-
-  // If the threads we're picking categories for change, (like when they
-  // get their categories updated), we expect our parents to pass us new
-  // props. We don't listen to the DatabaseStore ourselves.
-  componentWillReceiveProps(nextProps) {
-    this._account = AccountStore.accountForItems(nextProps.items);
-  }
-
-  _keymapHandlers() {
-    return { 'core:change-folders': this._onOpenCategoryPopover };
-  }
-
-  _onOpenCategoryPopover = () => {
-    if (!(this.props.items.length > 0)) {
-      return;
-    }
-    if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) {
-      return;
-    }
-    const buttonRect = this._buttonEl.getBoundingClientRect();
-    Actions.openPopover(<MovePickerPopover threads={this.props.items} account={this._account} />, {
-      originRect: buttonRect,
-      direction: 'down',
-    });
-  };
-
-  render() {
-    if (!this._account) {
-      return <span />;
-    }
-    const btnClasses = 'btn btn-toolbar btn-category-picker';
-
-    return (
-      <KeyCommandsRegion
-        style={{ order: -103 }}
-        globalHandlers={this._keymapHandlers()}
-        globalMenuItems={[
-          {
-            label: 'Thread',
-            submenu: [
-              {
-                label: 'Move to Folder...',
-                command: 'core:change-folders',
-                position: 'endof=thread-actions',
-              },
-            ],
-          },
-        ]}
-      >
-        <button
-          tabIndex={-1}
-          ref={el => (this._buttonEl = el)}
-          title={'Move to Folder'}
-          onClick={this._onOpenCategoryPopover}
-          className={btnClasses}
-        >
-          <RetinaImg name={'toolbar-movetofolder.png'} mode={RetinaImg.Mode.ContentIsMask} />
-        </button>
-      </KeyCommandsRegion>
-    );
-  }
-}
-
-module.exports = MovePicker;

app/internal_packages/category-picker/lib/toolbar-category-picker.jsx

@@ -0,0 +1,114 @@
+const { Actions, React, PropTypes, AccountStore, WorkspaceStore } = require('mailspring-exports');
+const { RetinaImg, KeyCommandsRegion } = require('mailspring-component-kit');
+
+const MovePickerPopover = require('./move-picker-popover').default;
+const LabelPickerPopover = require('./label-picker-popover').default;
+
+// This sets the folder / label on one or more threads.
+class MovePicker extends React.Component {
+  static displayName = 'MovePicker';
+  static containerRequired = false;
+
+  static propTypes = { items: PropTypes.array };
+  static contextTypes = { sheetDepth: PropTypes.number };
+
+  constructor(props) {
+    super(props);
+
+    this._account = AccountStore.accountForItems(this.props.items);
+  }
+
+  // If the threads we're picking categories for change, (like when they
+  // get their categories updated), we expect our parents to pass us new
+  // props. We don't listen to the DatabaseStore ourselves.
+  componentWillReceiveProps(nextProps) {
+    this._account = AccountStore.accountForItems(nextProps.items);
+  }
+
+  _onOpenLabelsPopover = () => {
+    if (!(this.props.items.length > 0)) {
+      return;
+    }
+    if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) {
+      return;
+    }
+    Actions.openPopover(<LabelPickerPopover threads={this.props.items} account={this._account} />, {
+      originRect: this._labelEl.getBoundingClientRect(),
+      direction: 'down',
+    });
+  };
+
+  _onOpenMovePopover = () => {
+    if (!(this.props.items.length > 0)) {
+      return;
+    }
+    if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) {
+      return;
+    }
+    Actions.openPopover(<MovePickerPopover threads={this.props.items} account={this._account} />, {
+      originRect: this._moveEl.getBoundingClientRect(),
+      direction: 'down',
+    });
+  };
+
+  render() {
+    if (!this._account) {
+      return <span />;
+    }
+
+    const handlers = {
+      'core:change-folders': this._onOpenMovePopover,
+    };
+    const submenu = [
+      {
+        label: 'Move to Folder...',
+        command: 'core:change-folders',
+        position: 'endof=thread-actions',
+      },
+    ];
+
+    if (this._account.usesLabels()) {
+      Object.assign(handlers, {
+        'core:change-labels': this._onOpenLabelsPopover,
+      });
+
+      submenu.push({
+        label: 'Apply Labels...',
+        command: 'core:change-labels',
+        position: 'endof=thread-actions',
+      });
+    }
+
+    return (
+      <div className="button-group" style={{ order: -103 }}>
+        <KeyCommandsRegion
+          globalHandlers={handlers}
+          globalMenuItems={[{ label: 'Thread', submenu: submenu }]}
+        >
+          <button
+            tabIndex={-1}
+            ref={el => (this._moveEl = el)}
+            title={'Move to Folder'}
+            onClick={this._onOpenMovePopover}
+            className={'btn btn-toolbar btn-category-picker'}
+          >
+            <RetinaImg name={'toolbar-movetofolder.png'} mode={RetinaImg.Mode.ContentIsMask} />
+          </button>
+          {this._account.usesLabels() && (
+            <button
+              tabIndex={-1}
+              ref={el => (this._labelEl = el)}
+              title={'Apply Labels'}
+              onClick={this._onOpenLabelsPopover}
+              className={'btn btn-toolbar btn-category-picker'}
+            >
+              <RetinaImg name={'toolbar-tag.png'} mode={RetinaImg.Mode.ContentIsMask} />
+            </button>
+          )}
+        </KeyCommandsRegion>
+      </div>
+    );
+  }
+}
+
+module.exports = MovePicker;

app/internal_packages/thread-list/lib/main.es6

@@ -6,15 +6,7 @@ import ThreadListEmptyFolderBar from './thread-list-empty-folder-bar';
 import MessageListToolbar from './message-list-toolbar';
 import SelectedItemsStack from './selected-items-stack';
 
-import {
+import { UpButton, DownButton, MoveButtons, FlagButtons } from './thread-toolbar-buttons';
-  UpButton,
-  DownButton,
-  TrashButton,
-  ArchiveButton,
-  MarkAsSpamButton,
-  ToggleUnreadButton,
-  ToggleStarredButton,
-} from './thread-toolbar-buttons';
 
 export function activate() {
   ComponentRegistry.register(ThreadListEmptyFolderBar, {
@@ -50,23 +42,11 @@ export function activate() {
     modes: ['list'],
   });
 
-  ComponentRegistry.register(ArchiveButton, {
+  ComponentRegistry.register(MoveButtons, {
     role: 'ThreadActionsToolbarButton',
   });
 
-  ComponentRegistry.register(TrashButton, {
+  ComponentRegistry.register(FlagButtons, {
-    role: 'ThreadActionsToolbarButton',
-  });
-
-  ComponentRegistry.register(MarkAsSpamButton, {
-    role: 'ThreadActionsToolbarButton',
-  });
-
-  ComponentRegistry.register(ToggleStarredButton, {
-    role: 'ThreadActionsToolbarButton',
-  });
-
-  ComponentRegistry.register(ToggleUnreadButton, {
     role: 'ThreadActionsToolbarButton',
   });
 }
@@ -76,11 +56,8 @@ export function deactivate() {
   ComponentRegistry.unregister(SelectedItemsStack);
   ComponentRegistry.unregister(ThreadListToolbar);
   ComponentRegistry.unregister(MessageListToolbar);
-  ComponentRegistry.unregister(ArchiveButton);
+  ComponentRegistry.unregister(MoveButtons);
-  ComponentRegistry.unregister(TrashButton);
+  ComponentRegistry.unregister(FlagButtons);
-  ComponentRegistry.unregister(MarkAsSpamButton);
-  ComponentRegistry.unregister(ToggleUnreadButton);
-  ComponentRegistry.unregister(ToggleStarredButton);
   ComponentRegistry.unregister(UpButton);
   ComponentRegistry.unregister(DownButton);
 }

app/internal_packages/thread-list/lib/thread-toolbar-buttons.jsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
-import { RetinaImg } from 'mailspring-component-kit';
+import { RetinaImg, CreateButtonGroup } from 'mailspring-component-kit';
 import {
   Actions,
   TaskFactory,
@@ -33,17 +33,11 @@ export class ArchiveButton extends React.Component {
   render() {
     const allowed = FocusedPerspectiveStore.current().canArchiveThreads(this.props.items);
     if (!allowed) {
-      return <span />;
+      return false;
     }
 
     return (
-      <button
+      <button tabIndex={-1} className="btn btn-toolbar" title="Archive" onClick={this._onArchive}>
-        tabIndex={-1}
-        style={{ order: -107 }}
-        className="btn btn-toolbar"
-        title="Archive"
-        onClick={this._onArchive}
-      >
         <RetinaImg name="toolbar-archive.png" mode={RetinaImg.Mode.ContentIsMask} />
       </button>
     );
@@ -72,13 +66,12 @@ export class TrashButton extends React.Component {
   render() {
     const allowed = FocusedPerspectiveStore.current().canMoveThreadsTo(this.props.items, 'trash');
     if (!allowed) {
-      return <span />;
+      return false;
     }
 
     return (
       <button
         tabIndex={-1}
-        style={{ order: -106 }}
         className="btn btn-toolbar"
         title="Move to Trash"
         onClick={this._onRemove}
@@ -129,7 +122,6 @@ export class MarkAsSpamButton extends React.Component {
       return (
         <button
           tabIndex={-1}
-          style={{ order: -105 }}
           className="btn btn-toolbar"
           title="Not Spam"
           onClick={this._onNotSpam}
@@ -141,12 +133,11 @@ export class MarkAsSpamButton extends React.Component {
 
     const allowed = FocusedPerspectiveStore.current().canMoveThreadsTo(this.props.items, 'spam');
     if (!allowed) {
-      return <span />;
+      return false;
     }
     return (
       <button
         tabIndex={-1}
-        style={{ order: -105 }}
         className="btn btn-toolbar"
         title="Mark as Spam"
         onClick={this._onMarkAsSpam}
@@ -182,13 +173,7 @@ export class ToggleStarredButton extends React.Component {
     const imageName = postClickStarredState ? 'toolbar-star.png' : 'toolbar-star-selected.png';
 
     return (
-      <button
+      <button tabIndex={-1} className="btn btn-toolbar" title={title} onClick={this._onStar}>
-        tabIndex={-1}
-        style={{ order: -103 }}
-        className="btn btn-toolbar"
-        title={title}
-        onClick={this._onStar}
-      >
         <RetinaImg name={imageName} mode={RetinaImg.Mode.ContentIsMask} />
       </button>
     );
@@ -222,7 +207,6 @@ export class ToggleUnreadButton extends React.Component {
     return (
       <button
         tabIndex={-1}
-        style={{ order: -104 }}
         className="btn btn-toolbar"
         title={`Mark as ${fragment}`}
         onClick={this._onClick}
@@ -284,6 +268,18 @@ class ThreadArrowButton extends React.Component {
   }
 }
 
+export const FlagButtons = CreateButtonGroup(
+  'FlagButtons',
+  [ToggleStarredButton, ToggleUnreadButton],
+  { order: -103 }
+);
+
+export const MoveButtons = CreateButtonGroup(
+  'MoveButtons',
+  [ArchiveButton, MarkAsSpamButton, TrashButton],
+  { order: -107 }
+);
+
 export const DownButton = () => {
   const getStateFromStores = () => {
     const selectedId = FocusedContentStore.focusedId('thread');

app/src/components/decorators/create-button-group.jsx

@@ -0,0 +1,13 @@
+import React from 'react';
+
+export default function CreateButtonGroup(name, buttons, { order = 0 }) {
+  const fn = props => {
+    return (
+      <div className="button-group" style={{ order }}>
+        {buttons.map(Component => <Component key={Component.displayName} {...props} />)}
+      </div>
+    );
+  };
+  fn.displayName = name;
+  return fn;
+}

app/src/global/mailspring-component-kit.es6

@@ -130,3 +130,4 @@ lazyLoad('ListensToObservable', 'decorators/listens-to-observable');
 lazyLoad('ListensToFluxStore', 'decorators/listens-to-flux-store');
 lazyLoad('ListensToMovementKeys', 'decorators/listens-to-movement-keys');
 lazyLoad('HasTutorialTip', 'decorators/has-tutorial-tip');
+lazyLoad('CreateButtonGroup', 'decorators/create-button-group');

app/static/workspace.less

@@ -269,6 +269,32 @@ body.is-blurred {
   .btn-toolbar:only-of-type {
     margin-right: @spacing-three-quarters;
   }
+
+  .button-group {
+    display: flex;
+    margin-left: @spacing-three-quarters;
+    .btn.btn-toolbar {
+      margin-left: 0;
+      margin-right: 0;
+
+      // Using these (slower) selectors to avoid redeclaring any constants
+      // like default-case border radius that themes might be overriding.
+      &:not(:last-child) {
+        border-right: 0;
+        border-top-right-radius: 0;
+        border-bottom-right-radius: 0;
+      }
+      &:not(:first-child) {
+        border-top-left-radius: 0;
+        border-bottom-left-radius: 0;
+      }
+      &:last-child {
+        border-left: 0;
+        border-top-left-radius: 0;
+        border-bottom-left-radius: 0;
+      }
+    }
+  }
 }
 
 .opacity-125ms-enter {