Rewrite gutter views in React

dmnd · dmnd · commit 2b9cbcbb20a9 · 2014-07-26T21:02:45.000-07:00
This will make it much easier to update the view after it becomes visible. This commit is just a rewrite though and I haven't added any functionality yet. This commit is dependent on atom/atom#3102 which allows me to workaround facebook/react#1939. I might vendorize a hacked version of React if those bugs take a while to be fixed. Also I noticed one regression - clicking in the editor seems to have a slight delay when the gutter is open. I'm not sure what's causing this but will try to fix. Test plan: I tested every piece of functionality I could think of, including: * Clicking on hash links * Scrolling * Opening when already scrolled * Adjusting width * Opening on a file that's not checked in (error still appears)
diff --git a/lib/controllers/blameViewController.js b/lib/controllers/blameViewController.js
@@ -1,5 +1,7 @@
 const _ = require('underscore');
 const $ = require('atom').$;
+// React depends on on https://github.com/atom/atom/pull/3102
+const React = require('atom').React;
 const BlameListView = require('../views/blame-list-view');
 const errorController = require('../controllers/errorController');
 
@@ -10,9 +12,9 @@ const TOGGLE_DEBOUNCE_TIME = 600;
  *
  * @return {JQuery} - The currently focused editor element.
  */
-function getFocusedEditorContainer () {
+function getFocusedEditorView () {
   var activePane = atom.workspaceView.getActivePaneView();
-  return activePane.find('.editor.is-focused');
+  return activePane.find('.editor.is-focused').view();
 }
 
 /**
@@ -24,21 +26,20 @@ function getFocusedEditorContainer () {
  * @param {Blamer} projectBlamer - a fully initialized Blamer for the current project
  */
 function toggleBlame(filePath, projectBlamer) {
-  var $focusedEditor = getFocusedEditorContainer();
-  if ($focusedEditor.hasClass('blaming')) {
-    // kill the blame container if we're already blaming this container
-    $focusedEditor.removeClass('blaming');
-    $focusedEditor.find('.git-blame').remove();
-
-    // unbind the scroll event
-    $focusedEditor.find('.vertical-scrollbar').off('scroll');
+  var focusedEditor = getFocusedEditorView();
+  if (focusedEditor.blameView) {
+    // we're already blaming this container, so unmount
+    focusedEditor.blameView = null;
+    var mountPoint = focusedEditor.find('.git-blame-mount');
+    React.unmountComponentAtNode(mountPoint);
+    mountPoint.remove();
   } else {
     // blame the given file + show view on success
-    projectBlamer.blame(filePath, function(err, blame) {
+    projectBlamer.blame(filePath, function(err, blameData) {
       if (err) {
         errorController.handleError(err);
       } else {
-        insertBlameView(blame, $focusedEditor);
+        insertBlameView(blameData, focusedEditor);
       }
     });
   }
@@ -51,17 +52,6 @@ function toggleBlame(filePath, projectBlamer) {
  */
 var debouncedToggleBlame = _.debounce(toggleBlame, TOGGLE_DEBOUNCE_TIME, true);
 
-/**
- * Makes the view arguments scroll position match the target elements scroll position
- *
- * @param {JQuery} view - the view whose scrollTop to adjust
- * @param {JQuery} target - element whose scrollTop should be matched
- */
-function matchScrollPosition(view, target) {
-  var targetScrollTop = target.scrollTop();
-  view.scrollTop(targetScrollTop);
-}
-
 /**
  * Inserts a BlameView rendered from input blameData into its proper
  * spot within the focusedEditor.
@@ -72,27 +62,20 @@ function matchScrollPosition(view, target) {
  *    the BlameView should be inserted
  */
 function insertBlameView(blameData, focusedEditor) {
-  var viewData = {
-    annotations: blameData
-  };
-  var blameView = new BlameListView(viewData);
-  var verticalScrollBar = focusedEditor.find('.vertical-scrollbar');
+  // insert the BlameListView after the gutter div
+  var mountPoint = $('<div>', {'class': 'git-blame-mount'});
+  focusedEditor.find('.gutter').after(mountPoint);
 
-  // Bind to scroll event on vertical-scrollbar for now to sync up scroll
-  // position of blame gutter.
-  verticalScrollBar.on('scroll', function(e) {
-    matchScrollPosition(blameView, $(e.target));
+  var blameView = new BlameListView({
+    annotations: blameData,
+    scrollbar: focusedEditor.find('.vertical-scrollbar')
   });
+  React.renderComponent(blameView, mountPoint[0]);
 
-  // insert the BlameListView after the gutter div
-  focusedEditor.find('.gutter').after(blameView);
-  focusedEditor.addClass('blaming');
-
-  // match scroll positions in case we blame at a scrolled position
-  matchScrollPosition(blameView, verticalScrollBar);
+  focusedEditor.blameView = blameView;
 }
 
 // EXPORTS
 module.exports = {
   toggleBlame: debouncedToggleBlame
-}
+};
diff --git a/lib/views/blame-line-view.coffee b/lib/views/blame-line-view.coffee
@@ -1,30 +1,27 @@
 {View} = require 'atom'
 RemoteRevision = require '../util/RemoteRevision'
 
-module.exports =
-class BlameLineView extends View
+{React, Reactionary} = require 'atom'
+{div, span, a} = Reactionary
 
-  @HASH_LENGTH: 7  # github uses this length
+HASH_LENGTH = 7  # github uses this length
 
-  @content: (params) ->
-    if params.noCommit
-      @div class: "blame-line no-commit text-subtle", =>
-        @span class: 'hash', '-'.repeat(@HASH_LENGTH)
-        @span class: 'date', params.date
-        @span class: 'committer', 'Nobody'
+module.exports =
+BlameLineComponent = React.createClass
+  render: ->
+    if @props.noCommit
+      div className: 'blame-line no-commit text-subtle',
+        span className: 'hash', '-'.repeat(HASH_LENGTH)
+        span className: 'date', @props.date
+        span className: 'committer', 'Nobody'
     else
-      @div class: 'blame-line ' + params.backgroundClass, =>
-        @a 'data-hash': params.hash, class: 'hash', click: 'hashClicked',
-           params.hash.substring(0, @HASH_LENGTH)
-        @span class: 'date', params.date
-        @span class: 'committer text-highlight',
-              params.committer.split(' ').slice(-1)[0]
-
-
-  hashClicked: (event, element) ->
-    filePath = atom.workspace.activePaneItem.getPath()
-    remoteUrl = atom.project.getRepo()?.getOriginUrl(filePath)
-    hash = element.data('hash')
+      url = RemoteRevision.create(@props.hash, @props.url).url()
+      div className: 'blame-line ' + @props.backgroundClass,
+        a className: 'hash', href: url,
+          @props.hash.substring(0, HASH_LENGTH)
+        span className: 'date', @props.date
+        span className: 'committer text-highlight',
+          @props.committer.split(' ').slice(-1)[0]
 
-    # create a RemoteRevision from hash/remoteUrl and open it
-    RemoteRevision.create(hash, remoteUrl).open()
+  shouldComponentUpdate: ->
+    false
diff --git a/lib/views/blame-list-view.coffee b/lib/views/blame-list-view.coffee
@@ -1,52 +1,105 @@
 {$, ScrollView} = require 'atom'
-BlameLineView = require './blame-line-view'
+BlameLineComponent = require './blame-line-view'
+_ = require('underscore')
 
-module.exports =
-class BlameListView extends ScrollView
+{React, Reactionary} = require 'atom'
+{div, span, a} = Reactionary
 
-  @content: (params) ->
-    @div class: 'git-blame', =>
-      @div class: 'git-blame-resize-handle'
-      @div class: 'git-blame-scroller', outlet: 'scroller', =>
-        @div class: 'blame-lines', =>
-          for blameLine in params.annotations
-            blameLine.backgroundClass = @lineClass blameLine
-            do (blameLine) =>
-              @subview 'blame-line-' + blameLine.line, new BlameLineView(blameLine)
 
-  afterAttach: ->
-    @on 'mousedown', '.git-blame-resize-handle', @resizeStarted
+BlameListLinesComponent = React.createClass
+  render: ->
+    filePath = atom.workspace.activePaneItem.getPath()
+    remoteUrl = atom.project.getRepo()?.getOriginUrl(filePath)
 
-  @lastHash: ''
+    bgClass = null
+    lastHash = null
+    lines = for ann in @props.annotations
+      args = _.clone ann  # clone so we can modify it
 
-  @lastBgClass: ''
+      # url to open diff
+      args['url'] = remoteUrl
 
-  @lineClass: (lineData) ->
-    if lineData.noCommit
-      return ''
+      # alternating background colour by commit
+      bgClass = if ann.noCommit
+        ''
+      else if ann.hash is lastHash
+        bgClass
+      else if bgClass is 'line-bg-lighter'
+        'line-bg-darker'
+      else
+        'line-bg-lighter'
+      args['backgroundClass'] = bgClass
+      lastHash = ann.hash
 
-    if lineData.hash isnt @lastHash
-      @lastHash = lineData.hash
-      @lastBgClass = if @lastBgClass == 'line-bg-lighter' then 'line-bg-darker' else 'line-bg-lighter'
+      # finally, render the line
+      BlameLineComponent(args)
+    div null, lines
 
-    return @lastBgClass
+  shouldComponentUpdate: ->
+    false
 
-  resizeStarted: ({pageX}) =>
-    @initialPageX = pageX
-    @initialWidth = @width()
-    $(document).on 'mousemove', @resizeBlameListView
+
+BlameListView = React.createClass
+  getInitialState: ->
+    {
+      # TODO: get this from the parent component somehow?
+      scrollTop: @props.scrollbar.scrollTop()
+      # TODO: be intelligent about persisting this so it doesn't reset
+      width: 210
+    }
+
+  render: ->
+    div
+      className: 'git-blame'
+      style: width: @state.width,
+      div className: 'git-blame-resize-handle', onMouseDown: @resizeStarted
+      div className: 'git-blame-scroller',
+        div
+          className: 'blame-lines'
+          style: WebkitTransform: @getTransform()
+          BlameListLinesComponent annotations: @props.annotations
+
+  getTransform: ->
+    {scrollTop} = @state
+
+    # hardware acceleration causes rendering issues on resize, disabled for now
+    useHardwareAcceleration = false
+    if useHardwareAcceleration
+      "translate3d(0px, #{-scrollTop}px, 0px)"
+    else
+      "translate(0px, #{-scrollTop}px)"
+
+  componentDidMount: ->
+    # Bind to scroll event on vertical-scrollbar to sync up scroll position of
+    # blame gutter.
+    @props.scrollbar.on 'scroll', @matchScrollPosition
+
+  componentWillUnmount: ->
+    @props.scrollbar.off 'scroll', @matchScrollPosition
+
+  # Makes the view arguments scroll position match the target elements scroll
+  # position
+  matchScrollPosition: ->
+    @setState scrollTop: @props.scrollbar.scrollTop()
+
+  resizeStarted: ({pageX}) ->
+    @setState dragging: true, initialPageX: pageX, initialWidth: @state.width
+    $(document).on 'mousemove', @onResizeMouseMove
     $(document).on 'mouseup', @resizeStopped
 
-  resizeStopped: =>
-    $(document).off 'mousemove', @resizeBlameListView
+  resizeStopped: (e) ->
+    $(document).off 'mousemove', @onResizeMouseMove
     $(document).off 'mouseup', @resizeStopped
+    @setState dragging: false
+
+    e.stopPropagation()
+    e.preventDefault()
+
+  onResizeMouseMove: (e) ->
+    return @resizeStopped() unless @state.dragging and e.which is 1
+    @setState width: @state.initialWidth + e.pageX - @state.initialPageX
 
-  resizeBlameListView: ({pageX, which}) =>
-    return @resizeStopped() unless which is 1
-    @width(@initialWidth + pageX - @initialPageX)
+    e.stopPropagation()
+    e.preventDefault()
 
-  # Used to sync up editor scrolling with the blame view.
-  # This is passed down to the scroller because we don't want to scroll the
-  # resizing markup.
-  scrollTop: (value) =>
-    @scroller.scrollTop value
+module.exports = BlameListView
diff --git a/stylesheets/git-blame.less b/stylesheets/git-blame.less
@@ -7,49 +7,50 @@
 
 @git-blame-bg-color: @base-background-color;
 
-.git-blame {
+.git-blame-mount {
   position: relative;
-  width: 210px; // initial width. TODO: persist? config?
 
-  .git-blame-resize-handle {
-    .stretch(@left: auto);
-    width: 10px;
-    cursor: col-resize;
-    z-index: 1;
-  }
+  .git-blame {
+    .git-blame-resize-handle {
+      .stretch(@left: auto);
+      width: 10px;
+      cursor: col-resize;
+      z-index: 1;
+    }
 
-  .git-blame-scroller {
-    .stretch();
-    overflow: hidden;
-    background-color: @git-blame-bg-color;
-    border-right: 1px solid @pane-item-border-color;
-    border-left: 1px solid @pane-item-border-color;
+    .git-blame-scroller {
+      .stretch();
+      overflow: hidden;
+      background-color: @git-blame-bg-color;
+      border-right: 1px solid @pane-item-border-color;
+      border-left: 1px solid @pane-item-border-color;
 
-    .blame-lines {
-      padding-bottom: 30px;
-      white-space: nowrap;
+      .blame-lines {
+        padding-bottom: 30px;
+        white-space: nowrap;
 
-      .blame-line {
-        padding-left: 4px;
-        overflow: hidden;
-        text-overflow: ellipsis;
+        .blame-line {
+          padding-left: 4px;
+          overflow: hidden;
+          text-overflow: ellipsis;
 
-        &.line-bg-lighter {
-          background-color: lighten(@git-blame-bg-color, 4%);
-        }
+          &.line-bg-lighter {
+            background-color: lighten(@git-blame-bg-color, 4%);
+          }
 
-        &.line-bg-darker {
-          background-color: darken(@git-blame-bg-color, 4%);
-        }
+          &.line-bg-darker {
+            background-color: darken(@git-blame-bg-color, 4%);
+          }
 
-        .hash, .date {
-          font-size: 90%;
-          padding-right: @component-padding;
-        }
+          .hash, .date {
+            font-size: 90%;
+            padding-right: @component-padding;
+          }
 
-        &.no-commit {
-          .committer, .date {
-            display: none;
+          &.no-commit {
+            .committer, .date {
+              display: none;
+            }
           }
         }
       }
