Browse Source

Fixes #3539 - When replying, quote article content only

Romit Choudhary 3 years ago
parent
commit
86d65a571b

+ 68 - 9
app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee

@@ -127,16 +127,14 @@ class EmailReply extends App.Controller
     # get current body
     body = ui.el.closest('.ticketZoom').find('.article-add [data-name="body"]').html() || ''
 
-    # check if quote need to be added
+    # check if quote need to be added via user selection of content
     signaturePosition = 'bottom'
-    selected = App.ClipBoard.getSelected('html')
-    if selected
-      selected = App.Utils.htmlCleanup(selected).html()
-    if !selected
-      selected = App.ClipBoard.getSelected('text')
-      if selected
-        selected = App.Utils.textCleanup(selected)
-        selected = App.Utils.text2html(selected)
+
+    if !@hasUserSelectedContent(ui)
+      selected = ''
+    else
+      selected = @getSelectedContent(ui)
+      selected = @cleanUpHtmlSelection(selected)
 
     # full quote, if needed
     if !selected && article && App.Config.get('ui_ticket_zoom_article_email_full_quote')
@@ -170,6 +168,67 @@ class EmailReply extends App.Controller
 
     true
 
+  @cleanUpHtmlSelection: (selected) ->
+    if selected
+      cleaned_up = App.Utils.htmlCleanup(selected).html()
+
+      return cleaned_up if cleaned_up
+
+    text = App.ClipBoard.getSelected('text')
+    return App.Utils.text2html(text) if text
+
+    false
+
+  # Fixes Issue #3539 - When replying quote article content only
+  @getSelectedContent: (ui) ->
+    range          = window.getSelection().getRangeAt(0)
+    parentSelector = ui.el.closest('.ticket-article-item').attr('id')
+
+    return if !parentSelector
+
+    lastSelElem = $('#' + parentSelector + ' .richtext-content')[0]
+
+    startInsideArticle = @isInsideSelectionBoundary(range.startContainer, parentSelector)
+    endInsideArticle   = @isInsideSelectionBoundary(range.endContainer, parentSelector)
+
+    if !startInsideArticle && endInsideArticle
+      range.setStart(lastSelElem, 0)
+    else if startInsideArticle && !endInsideArticle
+      range.setEnd(lastSelElem, lastSelElem.childNodes.length)
+    else if @containsNode(lastSelElem)
+      range.setStart(lastSelElem, 0)
+      range.setEnd(lastSelElem, lastSelElem.childNodes.length)
+
+    App.ClipBoard.manuallyUpdateSelection()
+    App.ClipBoard.getSelected('html')
+
+  # checks if user has made any text selection
+  # checks if that text selection is inside article-content only
+  @hasUserSelectedContent: (ui) ->
+    selObject = App.ClipBoard.getSelectedObject()
+
+    if selObject.rangeCount > 0
+      # item on which reply is clicked
+      parentTicketArticleContainer = ui.el.closest('.ticket-article-item')
+      if parentTicketArticleContainer
+        parentSelector = parentTicketArticleContainer.attr('id')
+        range = selObject.getRangeAt(0)
+        return @isInsideSelectionBoundary(range.startContainer, parentSelector) || @isInsideSelectionBoundary(range.endContainer, parentSelector) || @containsNode($('#' + parentSelector + ' .richtext-content')[0])
+    else
+      return false
+        
+  @isInsideSelectionBoundary: (node, parentSelectorId) ->
+    hasParent = $(node).closest('#' + parentSelectorId + ' .richtext-content')
+    return hasParent && hasParent.attr('class') is 'richtext-content'
+
+  # Selection.containsNode is not supported in IE, hence check
+  @containsNode: (node) ->
+    selected = App.ClipBoard.getSelectedObject()
+    if typeof selected.containsNode == 'function'
+      return selected.containsNode(node, false)
+    else
+      return false
+
   @date_format: (date_string) ->
     options = {
       weekday: 'long'

+ 25 - 2
app/assets/javascripts/app/lib/app_post/clipboard.coffee

@@ -6,11 +6,21 @@ class App.ClipBoard
       _instance ?= new _Singleton
     _instance.bind(el)
 
+  @manuallyUpdateSelection: (type) ->
+    if _instance == undefined
+      _instance ?= new _Singleton
+    _instance.manuallyUpdateSelection(type)
+  
   @getSelected: (type) ->
     if _instance == undefined
       _instance ?= new _Singleton
     _instance.getSelected(type)
 
+  @getSelectedObject: (type) ->
+    if _instance == undefined
+      _instance ?= new _Singleton
+    _instance.getSelectedObject(type)
+
   @getSelectedLast: (type) ->
     if _instance == undefined
       _instance ?= new _Singleton
@@ -36,9 +46,11 @@ class _Singleton
     @selection =
       html: ''
       text: ''
+      sel: null
     @selectionLast =
       html: ''
       text: ''
+      sel: null
 
   # bind to fill selected text into
   bind: (el) ->
@@ -59,7 +71,7 @@ class _Singleton
     )
 
   _updateSelection: =>
-    for key in ['html', 'text']
+    for key in ['html', 'text', 'sel']
       @selection[key] = @_getSelected(key)
       if @selection[key]
         @selectionLast[key] = @selection[key]
@@ -86,12 +98,23 @@ class _Singleton
       for i in [1..sel.rangeCount]
         container.appendChild(sel.getRangeAt(i-1).cloneContents())
       html = container.innerHTML
-    html
+    
+    if type != 'sel'
+      html
+    else
+      sel
+
+  manuallyUpdateSelection: ->
+    @_updateSelection()
 
   # get current selection
   getSelected: (type) ->
     @selection[type]
 
+  # get current selection original object
+  getSelectedObject: ->
+    @selection['sel']
+
   # get latest selection
   getSelectedLast: (type) ->
     @selectionLast[type]

+ 97 - 2
spec/system/ticket/update/full_quote_header_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr
   let(:ticket_article) { create(:ticket_article, ticket: ticket, from: 'Example Name <asdf1@example.com>') }
   let(:customer) { create(:customer) }
   let(:current_user) { customer }
+  let(:selection) { '' }
 
   prepend_before do
     Setting.set 'ui_ticket_zoom_article_email_full_quote_header', full_quote_header_setting
@@ -107,6 +108,78 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr
     end
   end
 
+  context 'when text is selected on page while replying' do
+    let(:full_quote_header_setting) { false }
+    let(:before_article_content_selector) { '.ticketZoom-header' }
+    let(:after_article_content_selector) { '.ticket-article-item .humanTimeFromNow' }
+    let(:article_content_selector) { '.ticket-article-item .richtext-content' }
+
+    it 'does not quote article when bits other than the article are selected' do
+      within(:active_content) do
+        selection = highlight_and_get_selection(before_article_content_selector, '')
+        click_reply
+
+        within(:richtext) do
+          expect(page).to have_no_text(selection)
+        end
+      end
+    end
+
+    it 'quotes article when bits inside the article are selected' do
+      within(:active_content) do
+        selection = highlight_and_get_selection(article_content_selector, '')
+        click_reply
+
+        within(:richtext) do
+          expect(page).to have_text(selection)
+        end
+      end
+    end
+
+    it 'quotes only article when bits before the article are selected as well' do
+      within(:active_content) do
+        selection = highlight_and_get_selection(before_article_content_selector, article_content_selector)
+        expected_text = find(article_content_selector).text
+
+        click_reply
+
+        within(:richtext) do
+          expect(page).to have_no_text(selection)
+          expect(page).to have_text(expected_text)
+        end
+      end
+    end
+
+    it 'quotes only article when bits after the article are selected as well' do
+      within(:active_content) do
+        selection = highlight_and_get_selection(article_content_selector, after_article_content_selector)
+        expected_text = find(article_content_selector).text
+
+        click_reply
+
+        within(:richtext) do
+          expect(page).to have_no_text(selection)
+          expect(page).to have_text(expected_text)
+        end
+      end
+    end
+
+    it 'quotes only article when bits both before and after the article are selected as well' do
+      within(:active_content) do
+        selection = highlight_and_get_selection(before_article_content_selector, after_article_content_selector)
+        expected_text = find(article_content_selector).text
+
+        click_reply
+
+        within(:richtext) do
+          expect(page).to have_no_text(selection)
+          expect(page).to have_text(expected_text)
+        end
+      end
+    end
+
+  end
+
   def click_forward
     click '.js-ArticleAction[data-type=emailForward]'
   end
@@ -115,8 +188,30 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr
     click '.js-ArticleAction[data-type=internal]'
   end
 
+  def click_reply
+    click '.js-ArticleAction[data-type=emailReply]'
+  end
+
+  def highlight_and_get_selection(start_selector, end_selector)
+    find(start_selector)
+      .execute_script(<<~JAVASCRIPT, end_selector)
+        let [ end_selector ] = arguments
+        let end_node = $(end_selector)[0]
+        if(!end_node) {
+          end_node = this.nextSibling
+        }
+        window.getSelection().removeAllRanges()
+        var range = window.document.createRange()
+        range.setStart(this, 0)
+        range.setEnd(end_node, end_node.childNodes.length)
+        window.getSelection().addRange(range)
+      JAVASCRIPT
+
+    find(start_selector).evaluate_script 'window.getSelection().toString().trim()'
+  end
+
   def highlight_and_click_reply
-    find('.ticket-article-item .textBubble')
+    find('.ticket-article-item .richtext-content')
       .execute_script <<~JAVASCRIPT
         window.getSelection().removeAllRanges()
         var range = window.document.createRange()
@@ -125,7 +220,7 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr
         window.getSelection().addRange(range)
       JAVASCRIPT
 
-    click '.js-ArticleAction[data-type=emailReply]'
+    click_reply
   end
 
   define :contain_full_quote do