From af96ec29f6249b1d3b7201282d5f928cc93b6f19 Mon Sep 17 00:00:00 2001 From: Desmond Brand Date: Fri, 29 Aug 2014 00:58:49 -0700 Subject: [PATCH 1/3] Update blame view upon save Fixes #29. Test plan: First test editing when view is open: * Open blame * Add some newlines * Save file * See blame view update Now test it updates upon second display: * Open blame * Close blame * Add some newlines * Save file * Open blame again * See it update --- lib/controllers/blameViewController.js | 4 +- lib/views/blame-line-view.coffee | 3 -- lib/views/blame-list-view.coffee | 59 +++++++++++++++++--------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/lib/controllers/blameViewController.js b/lib/controllers/blameViewController.js index 75f64ed..a988bc9 100644 --- a/lib/controllers/blameViewController.js +++ b/lib/controllers/blameViewController.js @@ -36,9 +36,7 @@ function toggleBlame(projectBlamer) { editorView.blameView = React.renderComponent(new BlameListView({ projectBlamer: projectBlamer, remoteRevision: remoteRevision, - filePath: filePath, - lineCount: editor.getLineCount(), - scrollbar: editorView.find('.vertical-scrollbar') + editorView: editorView }), mountPoint[0]); } else { editorView.blameView.toggle(); diff --git a/lib/views/blame-line-view.coffee b/lib/views/blame-line-view.coffee index 0777ae7..8c6b3f9 100644 --- a/lib/views/blame-line-view.coffee +++ b/lib/views/blame-line-view.coffee @@ -47,7 +47,4 @@ BlameLineComponent = React.createClass componentWillUnmount: -> $(@getDOMNode()).tooltip "destroy" - shouldComponentUpdate: -> - false - module.exports = {BlameLineComponent, renderLoading} diff --git a/lib/views/blame-list-view.coffee b/lib/views/blame-list-view.coffee index 14b3f07..c5f3ea9 100644 --- a/lib/views/blame-list-view.coffee +++ b/lib/views/blame-list-view.coffee @@ -1,5 +1,5 @@ {React, Reactionary, $} = require 'atom' -{div, span, a} = Reactionary +{div} = Reactionary RP = React.PropTypes _ = require 'underscore' {BlameLineComponent, renderLoading} = require './blame-line-view' @@ -9,12 +9,11 @@ BlameListLinesComponent = React.createClass propTypes: annotations: RP.arrayOf(RP.object) loading: RP.bool.isRequired - filePath: RP.string.isRequired - lineCount: RP.number.isRequired + initialLineCount: RP.number.isRequired remoteRevision: RP.object.isRequired renderLoading: -> - lines = [0...@props.lineCount].map renderLoading + lines = [0...@props.initialLineCount].map renderLoading div null, lines # makes background color alternate by commit @@ -57,20 +56,25 @@ BlameListView = React.createClass propTypes: projectBlamer: RP.object.isRequired remoteRevision: RP.object.isRequired - filePath: RP.string.isRequired - lineCount: RP.number.isRequired - scrollbar: RP.object.isRequired + editorView: RP.object.isRequired getInitialState: -> { # TODO: get this from the parent component somehow? - scrollTop: @props.scrollbar.scrollTop() + scrollTop: @scrollbar().scrollTop() # TODO: be intelligent about persisting this so it doesn't reset width: 210 loading: true visible: true + dirty: false } + scrollbar: -> + @_scrollbar ?= @props.editorView.find('.vertical-scrollbar') + + editor: -> + @_editor ?= @props.editorView.getModel() + render: -> display = if @state.visible then 'inline-block' else 'none' @@ -84,9 +88,8 @@ BlameListView = React.createClass style: WebkitTransform: @getTransform() BlameListLinesComponent annotations: @state.annotations - loading: @state.loading - filePath: @props.filePath - lineCount: @props.lineCount + loading: @state.loading and not @state.dirty + initialLineCount: @editor().getLineCount() remoteRevision: @props.remoteRevision div @@ -107,38 +110,54 @@ BlameListView = React.createClass componentWillMount: -> # kick off async request for blame data - @loadBlame true - - loadBlame: (force) -> - return if @state.loading and not force + @loadBlame() + @editor().on 'contents-modified', @contentsModified + @editor().buffer.on 'saved', @saved + loadBlame: -> @setState loading: true - @props.projectBlamer.blame @props.filePath, (err, data) => + @props.projectBlamer.blame @editor().getPath(), (err, data) => if err @setState loading: false error: true + dirty: false else @setState loading: false error: false + dirty: false annotations: data + contentsModified: -> + return unless @isMounted() + @setState dirty: true unless @state.dirty + + saved: -> + return unless @isMounted() + @loadBlame() if @state.visible and @state.dirty + toggle: -> - @setState visible: !@state.visible + if @state.visible + @setState visible: false + else + @loadBlame() if @state.dirty + @setState visible: true componentDidMount: -> # Bind to scroll event on vertical-scrollbar to sync up scroll position of # blame gutter. - @props.scrollbar.on 'scroll', @matchScrollPosition + @scrollbar().on 'scroll', @matchScrollPosition componentWillUnmount: -> - @props.scrollbar.off 'scroll', @matchScrollPosition + @scrollbar().off 'scroll', @matchScrollPosition + @editor().off 'contents-modified', @contentsModified + @editor().buffer.off 'saved', @saved # Makes the view arguments scroll position match the target elements scroll # position matchScrollPosition: -> - @setState scrollTop: @props.scrollbar.scrollTop() + @setState scrollTop: @scrollbar().scrollTop() resizeStarted: ({pageX}) -> @setState dragging: true, initialPageX: pageX, initialWidth: @state.width From 692fbf114a303cbcc520785495757812a8c33a6e Mon Sep 17 00:00:00 2001 From: Desmond Brand Date: Mon, 1 Sep 2014 23:27:36 -0700 Subject: [PATCH 2/3] Actually update blame view upon save In the last commit I think I simplified the code and tricked myself into thinking I tested it, but I must have not reloaded the package. This commit works though. Test plan: First test editing when view is open: * Open blame * Add some newlines * Save file * See blame view update Now test it updates upon second display: * Open blame * Close blame * Add some newlines * Save file * Open blame again * See it update --- lib/views/blame-line-view.coffee | 3 +++ lib/views/blame-list-view.coffee | 12 +++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/views/blame-line-view.coffee b/lib/views/blame-line-view.coffee index 8539d38..f71c693 100644 --- a/lib/views/blame-line-view.coffee +++ b/lib/views/blame-line-view.coffee @@ -50,4 +50,7 @@ BlameLineComponent = React.createClass componentWillUnmount: -> $(@getDOMNode()).tooltip "destroy" + shouldComponentUpdate: ({hash}) -> + hash isnt @props.hash + module.exports = {BlameLineComponent, renderLoading} diff --git a/lib/views/blame-list-view.coffee b/lib/views/blame-list-view.coffee index c5f3ea9..7b78815 100644 --- a/lib/views/blame-list-view.coffee +++ b/lib/views/blame-list-view.coffee @@ -9,6 +9,7 @@ BlameListLinesComponent = React.createClass propTypes: annotations: RP.arrayOf(RP.object) loading: RP.bool.isRequired + dirty: RP.bool.isRequired initialLineCount: RP.number.isRequired remoteRevision: RP.object.isRequired @@ -48,9 +49,10 @@ BlameListLinesComponent = React.createClass else @renderLoaded() - shouldComponentUpdate: ({loading}) -> - loading isnt @props.loading - + shouldComponentUpdate: ({loading, dirty}) -> + finishedInitialLoad = @props.loading and not loading and not @props.dirty + finishedEdit = @props.dirty and not dirty + finishedInitialLoad or finishedEdit BlameListView = React.createClass propTypes: @@ -88,10 +90,10 @@ BlameListView = React.createClass style: WebkitTransform: @getTransform() BlameListLinesComponent annotations: @state.annotations - loading: @state.loading and not @state.dirty + loading: @state.loading + dirty: @state.dirty initialLineCount: @editor().getLineCount() remoteRevision: @props.remoteRevision - div className: 'git-blame' style: width: @state.width, display: display From 26e3e1c8956bc697fc66776385182a475c51e0b6 Mon Sep 17 00:00:00 2001 From: Desmond Brand Date: Mon, 1 Sep 2014 23:46:12 -0700 Subject: [PATCH 3/3] Fix default date format string at startup Not sure what happened when I tried to do this in 39133b1, but evaluating `DEFAULT_DATE` at import time doesn't work because `atom.config.get` returns undefined. So calculate it later and cache it. Test Plan: Reloaded window then opened blame in a big file. Saw my preferred datestring and not the full ISO 8601 format. --- lib/views/blame-line-view.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/views/blame-line-view.coffee b/lib/views/blame-line-view.coffee index f71c693..8636014 100644 --- a/lib/views/blame-line-view.coffee +++ b/lib/views/blame-line-view.coffee @@ -6,13 +6,16 @@ moment = require 'moment' HASH_LENGTH = 7 # github uses this length BLANK_HASH = '-'.repeat(HASH_LENGTH) -DEFAULT_DATE = formatDate moment("2000-01-01T13:17:00 Z") + +_defaultDate = null +getDefaultDate = -> + _defaultDate ?= formatDate moment("2014-01-01T13:37:00 Z") renderLoading = -> div className: 'blame-line loading', span className: 'hash', BLANK_HASH - span className: 'date', DEFAULT_DATE + span className: 'date', getDefaultDate() span className: 'committer', 'Loading'