From 3d99ffc862b76c4dcc560aa8d8f4f9c1b7dd3f81 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Mar 2016 11:33:55 +0000 Subject: [PATCH 1/6] Edited timeago text on comments Unified the 'edited text' to be the same in descriptions and comments Closes #5538 --- app/assets/javascripts/notes.js.coffee | 1 + app/assets/stylesheets/pages/issuable.scss | 8 +++++ app/helpers/application_helper.rb | 30 ++++++++++++++----- app/views/projects/issues/show.html.haml | 5 +--- .../merge_requests/show/_mr_box.html.haml | 5 +--- app/views/projects/notes/_note.html.haml | 10 +------ 6 files changed, 35 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index c95ead22e6..063304ce89 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -328,6 +328,7 @@ class @Notes updateNote: (_xhr, note, _status) => # Convert returned HTML to a jQuery object so we can modify it further $html = $(note.html) + $('.js-timeago', $html).timeago() $html.syntaxHighlight() $html.find('.js-task-list-container').taskList('enable') diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 1310e6ad7c..e417869cc0 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -262,3 +262,11 @@ color: $gray-darkest; } } + +.edited-text { + color: $gray-darkest; + + .author_link { + color: $gray-darkest; + } +} diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 368969c647..cc4d2a8877 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -182,20 +182,36 @@ module ApplicationHelper # Returns an HTML-safe String def time_ago_with_tooltip(time, placement: 'top', html_class: 'time_ago', skip_js: false) element = content_tag :time, time.to_s, - class: "#{html_class} js-timeago js-timeago-pending", + class: "#{html_class} js-timeago", datetime: time.to_time.getutc.iso8601, title: time.in_time_zone.to_s(:medium), data: { toggle: 'tooltip', placement: placement, container: 'body' } - unless skip_js - element << javascript_tag( - "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" - ) - end - element end + def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', skip_js: false, include_author: false) + return nil if object.updated_at == object.created_at + + content_tag :small, class: "edited-text" do + output = content_tag :span do + "Edited " + end + output += time_ago_with_tooltip(object.updated_at) + + if include_author + if object.updated_by && object.updated_by != object.author + output += content_tag :span do + " by " + end + output += link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) + end + end + + output + end + end + def render_markup(file_name, file_content) if gitlab_markdown?(file_name) Haml::Helpers.preserve(markdown(file_content)) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 617b043780..f507fd7f5b 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -63,10 +63,7 @@ = markdown(@issue.description, cache_key: [@issue, "description"]) %textarea.hidden.js-task-list-field = @issue.description - - if @issue.updated_at != @issue.created_at - %small - Edited - = time_ago_with_tooltip(@issue.updated_at, placement: 'bottom', html_class: 'issue_edited_ago') + = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') .merge-requests = render 'merge_requests' diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml index 602f787e6c..a23bd8d18d 100644 --- a/app/views/projects/merge_requests/show/_mr_box.html.haml +++ b/app/views/projects/merge_requests/show/_mr_box.html.haml @@ -11,7 +11,4 @@ %textarea.hidden.js-task-list-field = @merge_request.description - - if @merge_request.updated_at != @merge_request.created_at - %small - Edited - = time_ago_with_tooltip(@merge_request.updated_at, placement: 'bottom') + = edited_time_ago_with_tooltip(@merge_request, placement: 'bottom') diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 52972576af..2cf32e6093 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -27,20 +27,13 @@ %span.note-last-update %a{name: dom_id(note), href: "##{dom_id(note)}", title: 'Link here'} = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note_created_ago') - - if note.updated_at != note.created_at - %span.note-updated-at - · - = icon('edit', title: 'edited') - = time_ago_with_tooltip(note.updated_at, placement: 'bottom', html_class: 'note_edited_ago') - - if note.updated_by && note.updated_by != note.author - by #{link_to_member(note.project, note.updated_by, avatar: false, author_class: nil)} - .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''} .note-text = preserve do = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) - if note_editable?(note) = render 'projects/notes/edit_form', note: note + = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note.attachment.url .note-attachment @@ -54,4 +47,3 @@ = link_to delete_attachment_namespace_project_note_path(note.project.namespace, note.project, note), title: 'Delete this attachment', method: :delete, remote: true, data: { confirm: 'Are you sure you want to remove the attachment?' }, class: 'danger js-note-attachment-delete' do = icon('trash-o', class: 'cred') - .clear From 538f3e0d716d0f8c107af030fb318808f9e284ac Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Mar 2016 15:07:45 +0000 Subject: [PATCH 2/6] Fixed ruby issues from feedback Fixed failing tests --- app/helpers/application_helper.rb | 19 +++++++------------ app/views/projects/_last_commit.html.haml | 2 +- app/views/projects/blame/show.html.haml | 2 +- app/views/projects/commits/_commit.html.haml | 2 +- spec/helpers/application_helper_spec.rb | 15 ++------------- 5 files changed, 12 insertions(+), 28 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index cc4d2a8877..f99a14c0e0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -168,7 +168,6 @@ module ApplicationHelper # time - Time object # placement - Tooltip placement String (default: "top") # html_class - Custom class for `time` element (default: "time_ago") - # skip_js - When true, exclude the `script` tag (default: false) # # By default also includes a `script` element with Javascript necessary to # initialize the `timeago` jQuery extension. If this method is called many @@ -180,7 +179,7 @@ module ApplicationHelper # `html_class` argument is provided. # # Returns an HTML-safe String - def time_ago_with_tooltip(time, placement: 'top', html_class: 'time_ago', skip_js: false) + def time_ago_with_tooltip(time, placement: 'top', html_class: 'time_ago') element = content_tag :time, time.to_s, class: "#{html_class} js-timeago", datetime: time.to_time.getutc.iso8601, @@ -190,21 +189,17 @@ module ApplicationHelper element end - def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', skip_js: false, include_author: false) - return nil if object.updated_at == object.created_at + def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', include_author: false) + return if object.updated_at == object.created_at content_tag :small, class: "edited-text" do - output = content_tag :span do - "Edited " - end - output += time_ago_with_tooltip(object.updated_at) + output = content_tag(:span, "Edited ") + output << time_ago_with_tooltip(object.updated_at, placement: placement, html_class: html_class) if include_author if object.updated_by && object.updated_by != object.author - output += content_tag :span do - " by " - end - output += link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) + output << content_tag(:span, " by ") + output << link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) end end diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml index 386d72e778..a47aea7ff1 100644 --- a/app/views/projects/_last_commit.html.haml +++ b/app/views/projects/_last_commit.html.haml @@ -8,5 +8,5 @@ = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id" = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message" · - #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} by + #{time_ago_with_tooltip(commit.committed_date)} by = commit_author_link(commit, avatar: true, size: 24) diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index 5f9a92ff93..1ae83c2383 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -29,7 +29,7 @@ .light = commit_author_link(commit, avatar: false) authored - #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} + #{time_ago_with_tooltip(commit.committed_date)} %td.line-numbers - line_count = blame_group[:lines].count - (current_line...(current_line + line_count)).each do |i| diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 7f2903589a..4df4a8ca9e 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -38,5 +38,5 @@ = commit_author_link(commit, avatar: true, size: 24) authored .committed_ago - #{time_ago_with_tooltip(commit.committed_date, skip_js: true)}   + #{time_ago_with_tooltip(commit.committed_date)}   = link_to_browse_code(project, commit) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 8013b31524..2c27e6a031 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -263,29 +263,18 @@ describe ApplicationHelper do end it 'includes a default js-timeago class' do - expect(element.attr('class')).to eq 'time_ago js-timeago js-timeago-pending' + expect(element.attr('class')).to eq 'time_ago js-timeago' end it 'accepts a custom html_class' do expect(element(html_class: 'custom_class').attr('class')). - to eq 'custom_class js-timeago js-timeago-pending' + to eq 'custom_class js-timeago' end it 'accepts a custom tooltip placement' do expect(element(placement: 'bottom').attr('data-placement')).to eq 'bottom' end - it 're-initializes timeago Javascript' do - el = element.next_element - - expect(el.name).to eq 'script' - expect(el.text).to include "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" - end - - it 'allows the script tag to be excluded' do - expect(element(skip_js: true)).not_to include 'script' - end - it 'converts to Time' do expect { helper.time_ago_with_tooltip(Date.today) }.not_to raise_error end From 12506abfe794e75fcaa879b92bee09b8da57bf46 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Mar 2016 17:04:32 +0000 Subject: [PATCH 3/6] Single if statement --- app/helpers/application_helper.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f99a14c0e0..212fecc3e7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -196,11 +196,9 @@ module ApplicationHelper output = content_tag(:span, "Edited ") output << time_ago_with_tooltip(object.updated_at, placement: placement, html_class: html_class) - if include_author - if object.updated_by && object.updated_by != object.author - output << content_tag(:span, " by ") - output << link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) - end + if include_author && object.updated_by && object.updated_by != object.author + output << content_tag(:span, " by ") + output << link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) end output From cd1d045fe215bb06a1cc96e12cefda5ef71058a0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 9 Mar 2016 16:29:15 +0000 Subject: [PATCH 4/6] Fixed issue with timeago not firing --- app/helpers/application_helper.rb | 10 ++++++++-- app/views/projects/_last_commit.html.haml | 2 +- app/views/projects/blame/show.html.haml | 2 +- app/views/projects/commits/_commit.html.haml | 2 +- spec/helpers/application_helper_spec.rb | 15 +++++++++++++-- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 212fecc3e7..5cd494079b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -179,13 +179,19 @@ module ApplicationHelper # `html_class` argument is provided. # # Returns an HTML-safe String - def time_ago_with_tooltip(time, placement: 'top', html_class: 'time_ago') + def time_ago_with_tooltip(time, placement: 'top', html_class: 'time_ago', skip_js: false) element = content_tag :time, time.to_s, - class: "#{html_class} js-timeago", + class: "#{html_class} js-timeago #{"js-timeago-pending" unless skip_js}", datetime: time.to_time.getutc.iso8601, title: time.in_time_zone.to_s(:medium), data: { toggle: 'tooltip', placement: placement, container: 'body' } + unless skip_js + element << javascript_tag( + "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" + ) + end + element end diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml index a47aea7ff1..386d72e778 100644 --- a/app/views/projects/_last_commit.html.haml +++ b/app/views/projects/_last_commit.html.haml @@ -8,5 +8,5 @@ = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id" = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message" · - #{time_ago_with_tooltip(commit.committed_date)} by + #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} by = commit_author_link(commit, avatar: true, size: 24) diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index 1ae83c2383..5f9a92ff93 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -29,7 +29,7 @@ .light = commit_author_link(commit, avatar: false) authored - #{time_ago_with_tooltip(commit.committed_date)} + #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} %td.line-numbers - line_count = blame_group[:lines].count - (current_line...(current_line + line_count)).each do |i| diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 4df4a8ca9e..7f2903589a 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -38,5 +38,5 @@ = commit_author_link(commit, avatar: true, size: 24) authored .committed_ago - #{time_ago_with_tooltip(commit.committed_date)}   + #{time_ago_with_tooltip(commit.committed_date, skip_js: true)}   = link_to_browse_code(project, commit) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 2c27e6a031..8013b31524 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -263,18 +263,29 @@ describe ApplicationHelper do end it 'includes a default js-timeago class' do - expect(element.attr('class')).to eq 'time_ago js-timeago' + expect(element.attr('class')).to eq 'time_ago js-timeago js-timeago-pending' end it 'accepts a custom html_class' do expect(element(html_class: 'custom_class').attr('class')). - to eq 'custom_class js-timeago' + to eq 'custom_class js-timeago js-timeago-pending' end it 'accepts a custom tooltip placement' do expect(element(placement: 'bottom').attr('data-placement')).to eq 'bottom' end + it 're-initializes timeago Javascript' do + el = element.next_element + + expect(el.name).to eq 'script' + expect(el.text).to include "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" + end + + it 'allows the script tag to be excluded' do + expect(element(skip_js: true)).not_to include 'script' + end + it 'converts to Time' do expect { helper.time_ago_with_tooltip(Date.today) }.not_to raise_error end From c7730121c16e41d797e7f7e00f7a6ad96d78bbb4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 10 Mar 2016 08:39:32 +0000 Subject: [PATCH 5/6] Fixed failing rubocop test --- app/helpers/application_helper.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5cd494079b..f07c79ec61 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -186,11 +186,11 @@ module ApplicationHelper title: time.in_time_zone.to_s(:medium), data: { toggle: 'tooltip', placement: placement, container: 'body' } - unless skip_js - element << javascript_tag( - "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" - ) - end + unless skip_js + element << javascript_tag( + "$('.js-timeago-pending').removeClass('js-timeago-pending').timeago()" + ) + end element end From 2941a76fe0fa5305bd6e31e6c47477732e3e7cec Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 16 Mar 2016 08:51:09 +0000 Subject: [PATCH 6/6] Added skip_js comment [ci skip] --- app/helpers/application_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f07c79ec61..ffddd0400f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -168,6 +168,7 @@ module ApplicationHelper # time - Time object # placement - Tooltip placement String (default: "top") # html_class - Custom class for `time` element (default: "time_ago") + # skip_js - When true, exclude the `script` tag (default: false) # # By default also includes a `script` element with Javascript necessary to # initialize the `timeago` jQuery extension. If this method is called many