Merge branch 'rs-reply-hotkey' into 'master'

"Reply quoting selected text" shortcut/hotkey

Adds the <kbd>r</kbd> hotkey for quoting selected text on Issuable forms.

This MR also updates the jasmine gem and adds jasmine-rails to let us use the asset pipeline (and Coffeescript) in JS specs.

See merge request !1775

CHANGELOG

@@ -5,6 +5,7 @@ v 7.11.0 (unreleased)
   - Ignore invalid lines in .gitmodules
   - Fix "Cannot move project" error message from popping up after a successful transfer (Stan Hu)
   -
+  - Add "Reply quoting selected text" shortcut key (`r`)
   -
   -
   -

Gemfile

@@ -251,7 +251,8 @@ group :development, :test do
   # PhantomJS driver for Capybara
   gem 'poltergeist', '~> 1.5.1'
 
-  gem 'jasmine', '2.0.2'
+  gem 'jasmine', '~> 2.2.0'
+  gem 'jasmine-rails'
 
   gem "spring", '~> 1.3.1'
   gem "spring-commands-rspec", '1.0.4'

Gemfile.lock

@@ -287,12 +287,17 @@ GEM
     i18n (0.7.0)
     ice_cube (0.11.1)
     ice_nine (0.10.0)
-    jasmine (2.0.2)
-      jasmine-core (~> 2.0.0)
+    jasmine (2.2.0)
+      jasmine-core (~> 2.2)
       phantomjs
       rack (>= 1.2.1)
       rake
-    jasmine-core (2.0.0)
+    jasmine-core (2.2.0)
+    jasmine-rails (0.10.8)
+      jasmine-core (>= 1.3, < 3.0)
+      phantomjs (>= 1.9)
+      railties (>= 3.2.0)
+      sprockets-rails
     jquery-atwho-rails (0.3.3)
     jquery-rails (3.1.0)
       railties (>= 3.0, < 5.0)
@@ -387,7 +392,7 @@ GEM
     parser (2.2.0.2)
       ast (>= 1.1, < 3.0)
     pg (0.15.1)
-    phantomjs (1.9.2.0)
+    phantomjs (1.9.8.0)
     poltergeist (1.5.1)
       capybara (~> 2.1)
       cliver (~> 0.3.1)
@@ -713,7 +718,8 @@ DEPENDENCIES
   hipchat (~> 1.5.0)
   html-pipeline (~> 1.11.0)
   httparty
-  jasmine (= 2.0.2)
+  jasmine (~> 2.2.0)
+  jasmine-rails
   jquery-atwho-rails (~> 0.3.3)
   jquery-rails
   jquery-scrollto-rails

app/assets/javascripts/application.js.coffee

@@ -38,7 +38,7 @@
 #= require shortcuts
 #= require shortcuts_navigation
 #= require shortcuts_dashboard_navigation
-#= require shortcuts_issueable
+#= require shortcuts_issuable
 #= require shortcuts_network
 #= require cal-heatmap
 #= require_tree .

app/assets/javascripts/dispatcher.js.coffee

@@ -22,7 +22,7 @@ class Dispatcher
         shortcut_handler = new ShortcutsNavigation()
       when 'projects:issues:show'
         new Issue()
-        shortcut_handler = new ShortcutsIssueable()
+        shortcut_handler = new ShortcutsIssuable()
         new ZenMode()
       when 'projects:milestones:show'
         new Milestone()
@@ -47,7 +47,7 @@ class Dispatcher
           new IssuableForm($('.merge-request-form'))
       when 'projects:merge_requests:show'
         new Diff()
-        shortcut_handler = new ShortcutsIssueable()
+        shortcut_handler = new ShortcutsIssuable()
         new ZenMode()
       when "projects:merge_requests:diffs"
         new Diff()

app/assets/javascripts/shortcuts_issuable.coffee

+#= require jquery
+#= require mousetrap
+
+#= require shortcuts_navigation
+
+class @ShortcutsIssuable extends ShortcutsNavigation
+  constructor: (isMergeRequest) ->
+    super()
+    Mousetrap.bind('a', ->
+      $('.js-assignee').select2('open')
+      return false
+    )
+    Mousetrap.bind('m', ->
+      $('.js-milestone').select2('open')
+      return false
+    )
+    Mousetrap.bind('r', =>
+      @replyWithSelectedText()
+      return false
+    )
+
+    if isMergeRequest
+      @enabledHelp.push('.hidden-shortcut.merge_requests')
+    else
+      @enabledHelp.push('.hidden-shortcut.issues')
+
+  replyWithSelectedText: ->
+    if window.getSelection
+      selected = window.getSelection().toString()
+      replyField = $('.js-main-target-form #note_note')
+
+      return if selected.trim() == ""
+
+      # Put a '>' character before each non-empty line in the selection
+      quote = _.map selected.split("\n"), (val) ->
+        "> #{val}\n" if val.trim() != ''
+
+      # If replyField already has some content, add a newline before our quote
+      separator = replyField.val().trim() != "" and "\n" or ''
+
+      replyField.val (_, current) ->
+        current + separator + quote.join('') + "\n"
+
+      # Trigger autosave for the added text
+      replyField.trigger('input')
+
+      # Focus the input field
+      replyField.focus()

app/assets/javascripts/shortcuts_issueable.coffee

-#= require shortcuts_navigation
-
-class @ShortcutsIssueable extends ShortcutsNavigation
-  constructor: (isMergeRequest) ->
-    super()
-    Mousetrap.bind('a', ->
-      $('.js-assignee').select2('open')
-      return false
-    )
-    Mousetrap.bind('m', ->
-      $('.js-milestone').select2('open')
-      return false
-    )
-    
-    if isMergeRequest
-      @enabledHelp.push('.hidden-shortcut.merge_reuests')
-    else
-      @enabledHelp.push('.hidden-shortcut.issues')
-

app/assets/javascripts/stat_graph_contributors.js.coffee

+#= require d3
+#= require jquery
+#= require stat_graph_contributors_util
+
 class @ContributorsStatGraph
   init: (log) ->
     @parsed_log = ContributorsStatGraphUtil.parse_log(log)

app/assets/javascripts/stat_graph_contributors_graph.js.coffee

+#= require d3
+#= require jquery
+#= require underscore
+
 class @ContributorsGraph
   MARGIN:
     top: 20

app/views/help/_shortcuts.html.haml

@@ -187,7 +187,11 @@
                 %td.shortcut
                   .key m
                 %td Change milestone
-            %tbody{ class: 'hidden-shortcut merge_reuests', style: 'display:none' }
+              %tr
+                %td.shortcut
+                  .key r
+                %td Reply (quoting selected text)
+            %tbody{ class: 'hidden-shortcut merge_requests', style: 'display:none' }
               %tr
                 %th
                 %th Merge Requests
@@ -199,6 +203,10 @@
                 %td.shortcut
                   .key m
                 %td Change milestone
+              %tr
+                %td.shortcut
+                  .key r
+                %td Reply (quoting selected text)
 
 
 :javascript

config/routes.rb

@@ -2,6 +2,7 @@ require 'sidekiq/web'
 require 'api/api'
 
 Gitlab::Application.routes.draw do
+  mount JasmineRails::Engine => '/specs' if defined?(JasmineRails)
   use_doorkeeper do
     controllers applications: 'oauth/applications',
                 authorized_applications: 'oauth/authorized_applications',

spec/javascripts/shortcuts_issuable_spec.js.coffee

+#= require jquery
+#= require jasmine-fixture
+
+#= require shortcuts_issuable
+
+describe 'ShortcutsIssuable', ->
+  beforeEach ->
+    @shortcut = new ShortcutsIssuable()
+
+  describe '#replyWithSelectedText', ->
+    # Stub window.getSelection to return the provided String.
+    stubSelection = (text) ->
+      window.getSelection = -> text
+
+    beforeEach ->
+      @selector = 'form.js-main-target-form textarea#note_note'
+      affix(@selector)
+
+    describe 'with empty selection', ->
+      it 'does nothing', ->
+        stubSelection('')
+        @shortcut.replyWithSelectedText()
+        expect($(@selector).val()).toBe('')
+
+    describe 'with any selection', ->
+      beforeEach ->
+        stubSelection('Selected text.')
+
+      it 'leaves existing input intact', ->
+        $(@selector).val('This text was already here.')
+        expect($(@selector).val()).toBe('This text was already here.')
+
+        @shortcut.replyWithSelectedText()
+        expect($(@selector).val()).
+          toBe("This text was already here.\n> Selected text.\n\n")
+
+      it 'triggers `input`', ->
+        triggered = false
+        $(@selector).on 'input', -> triggered = true
+        @shortcut.replyWithSelectedText()
+
+        expect(triggered).toBe(true)
+
+      it 'triggers `focus`', ->
+        focused = false
+        $(@selector).on 'focus', -> focused = true
+        @shortcut.replyWithSelectedText()
+
+        expect(focused).toBe(true)
+
+    describe 'with a one-line selection', ->
+      it 'quotes the selection', ->
+        stubSelection('This text has been selected.')
+
+        @shortcut.replyWithSelectedText()
+
+        expect($(@selector).val()).
+          toBe("> This text has been selected.\n\n")
+
+    describe 'with a multi-line selection', ->
+      it 'quotes the selected lines as a group', ->
+        stubSelection(
+          """
+          Selected line one.
+
+          Selected line two.
+          Selected line three.
+
+          """
+        )
+
+        @shortcut.replyWithSelectedText()
+
+        expect($(@selector).val()).
+          toBe(
+            """
+            > Selected line one.
+            > Selected line two.
+            > Selected line three.
+
+
+            """
+          )

spec/javascripts/stat_graph_contributors_graph_spec.js

+//= require stat_graph_contributors_graph
+
 describe("ContributorsGraph", function () {
   describe("#set_x_domain", function () {
     it("set the x_domain", function () {

spec/javascripts/stat_graph_contributors_util_spec.js

+//= require stat_graph_contributors_util
+
 describe("ContributorsStatGraphUtil", function () {
 
   describe("#parse_log", function () {

spec/javascripts/stat_graph_spec.js

+//= require stat_graph
+
 describe("StatGraph", function () {
 
   describe("#get_log", function () {

spec/javascripts/support/jasmine.yml

-# src_files
+# path to parent directory of spec_files
+# relative path from Rails.root
 #
-# Return an array of filepaths relative to src_dir to include before jasmine specs.
-# Default: []
+# Alternatively accept an array of directory to include external spec files
+# spec_dir:
+#   - spec/javascripts
+#   - ../engine/spec/javascripts
 #
-# EXAMPLE:
-#
-# src_files:
-#   - lib/source1.js
-#   - lib/source2.js
-#   - dist/**/*.js
-#
-src_files:
-  - assets/application.js
-
-# stylesheets
-#
-# Return an array of stylesheet filepaths relative to src_dir to include before jasmine specs.
-# Default: []
-#
-# EXAMPLE:
-#
-# stylesheets:
-#   - css/style.css
-#   - stylesheets/*.css
-#
-stylesheets:
-  - stylesheets/**/*.css
+# defaults to spec/javascripts
+spec_dir: spec/javascripts
 
-# helpers
-#
-# Return an array of filepaths relative to spec_dir to include before jasmine specs.
-# Default: ["helpers/**/*.js"]
-#
-# EXAMPLE:
-#
-# helpers:
-#   - helpers/**/*.js
-#
+# list of file expressions to include as helpers into spec runner
+# relative path from spec_dir
 helpers:
-  - helpers/**/*.js
+  - "helpers/**/*.{js.coffee,js,coffee}"
 
-# spec_files
-#
-# Return an array of filepaths relative to spec_dir to include.
-# Default: ["**/*[sS]pec.js"]
-#
-# EXAMPLE:
-#
-# spec_files:
-#   - **/*[sS]pec.js
-#
+# list of file expressions to include as specs into spec runner
+# relative path from spec_dir
 spec_files:
-  - '**/*[sS]pec.js'
-
-# src_dir
-#
-# Source directory path. Your src_files must be returned relative to this path. Will use root if left blank.
-# Default: project root
-#
-# EXAMPLE:
-#
-# src_dir: public
-#
-src_dir:
-
-# spec_dir
-#
-# Spec directory path. Your spec_files must be returned relative to this path.
-# Default: spec/javascripts
-#
-# EXAMPLE:
-#
-# spec_dir: spec/javascripts
-#
-spec_dir: spec/javascripts
+  - "**/*[Ss]pec.{js.coffee,js,coffee}"

spec/javascripts/support/jasmine_helper.rb

@@ -8,4 +8,8 @@
 #   config.boot_files = lambda { ['/absolute/path/to/boot_dir/file.js'] }
 #end
 #
-
+#Example: prevent PhantomJS auto install, uses PhantomJS already on your path.
+#Jasmine.configure do |config|
+#   config.prevent_phantom_js_auto_install = true
+#end
+#