Commit cf55fb9b authored by Clement Ho's avatar Clement Ho

Merge branch '34527-make-edit-comment-button-always-available-outside-of-dropdown' into 'master'

Resolve "Make edit comment button always available outside of dropdown"

Closes #34527

See merge request !12931
parents 3248911c 8858ddaf
...@@ -453,7 +453,10 @@ ul.notes { ...@@ -453,7 +453,10 @@ ul.notes {
} }
.note-actions { .note-actions {
align-self: flex-start;
flex-shrink: 0; flex-shrink: 0;
display: inline-flex;
align-items: center;
// For PhantomJS that does not support flex // For PhantomJS that does not support flex
float: right; float: right;
margin-left: 10px; margin-left: 10px;
...@@ -463,18 +466,12 @@ ul.notes { ...@@ -463,18 +466,12 @@ ul.notes {
float: none; float: none;
margin-left: 0; margin-left: 0;
} }
.note-action-button {
margin-left: 8px;
}
.more-actions-toggle {
margin-left: 2px;
}
} }
.more-actions { .more-actions {
display: inline-block; float: right; // phantomjs fallback
display: flex;
align-items: flex-end;
.tooltip { .tooltip {
white-space: nowrap; white-space: nowrap;
...@@ -482,16 +479,10 @@ ul.notes { ...@@ -482,16 +479,10 @@ ul.notes {
} }
.more-actions-toggle { .more-actions-toggle {
padding: 0;
&:hover .icon, &:hover .icon,
&:focus .icon { &:focus .icon {
color: $blue-600; color: $blue-600;
} }
.icon {
padding: 0 6px;
}
} }
.more-actions-dropdown { .more-actions-dropdown {
...@@ -519,28 +510,42 @@ ul.notes { ...@@ -519,28 +510,42 @@ ul.notes {
@include notes-media('max', $screen-md-max) { @include notes-media('max', $screen-md-max) {
float: none; float: none;
margin-left: 0; margin-left: 0;
.note-action-button {
margin-left: 0;
} }
}
.note-actions-item {
margin-left: 15px;
display: flex;
align-items: center;
&.more-actions {
// compensate for narrow icon
margin-left: 10px;
} }
} }
.note-action-button { .note-action-button {
display: inline; line-height: 1;
line-height: 20px; padding: 0;
min-width: 16px;
color: $gray-darkest;
.fa { .fa {
color: $gray-darkest;
position: relative; position: relative;
font-size: 17px; font-size: 16px;
} }
svg { svg {
height: 16px; height: 16px;
width: 16px; width: 16px;
fill: $gray-darkest; top: 0;
vertical-align: text-top; vertical-align: text-top;
path {
fill: currentColor;
}
} }
.award-control-icon-positive, .award-control-icon-positive,
...@@ -613,10 +618,7 @@ ul.notes { ...@@ -613,10 +618,7 @@ ul.notes {
.note-role { .note-role {
position: relative; position: relative;
top: -2px; padding: 0 7px;
display: inline-block;
padding-left: 7px;
padding-right: 7px;
color: $notes-role-color; color: $notes-role-color;
font-size: 12px; font-size: 12px;
line-height: 20px; line-height: 20px;
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
"inline-template" => true, "inline-template" => true,
"ref" => "note_#{note.id}" } "ref" => "note_#{note.id}" }
.note-actions-item
%button.note-action-button.line-resolve-btn{ type: "button", %button.note-action-button.line-resolve-btn{ type: "button",
class: ("is-disabled" unless can_resolve), class: ("is-disabled" unless can_resolve),
":class" => "{ 'is-active': isResolved }", ":class" => "{ 'is-active': isResolved }",
...@@ -31,10 +32,17 @@ ...@@ -31,10 +32,17 @@
- if current_user - if current_user
- if note.emoji_awardable? - if note.emoji_awardable?
- user_authored = note.user_authored?(current_user) - user_authored = note.user_authored?(current_user)
= link_to '#', title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored} has-tooltip", data: { position: 'right' } do .note-actions-item
= button_tag title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored} has-tooltip btn btn-transparent", data: { position: 'right', container: 'body' } do
= icon('spinner spin') = icon('spinner spin')
%span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face') %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
%span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley') %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
%span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile') %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
- if note_editable
.note-actions-item
= button_tag title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip btn btn-transparent', data: { container: 'body' } do
%span.link-highlight
= custom_icon('icon_pencil')
= render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
- is_current_user = current_user == note.author - is_current_user = current_user == note.author
- if note_editable || !is_current_user - if note_editable || !is_current_user
.dropdown.more-actions .dropdown.more-actions.note-actions-item
= button_tag title: 'More actions', class: 'note-action-button more-actions-toggle has-tooltip btn btn-transparent', data: { toggle: 'dropdown', container: 'body' } do = button_tag title: 'More actions', class: 'note-action-button more-actions-toggle has-tooltip btn btn-transparent', data: { toggle: 'dropdown', container: 'body' } do
= icon('ellipsis-v', class: 'icon') %span.icon
= custom_icon('ellipsis_v')
%ul.dropdown-menu.more-actions-dropdown.dropdown-open-left %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left
- if note_editable
%li
= button_tag 'Edit comment', class: 'js-note-edit btn btn-transparent'
%li.divider
- unless is_current_user - unless is_current_user
%li %li
= link_to new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) do = link_to new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) do
......
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1600 1600"><path d="M1088 1248v192q0 40-28 68t-68 28H800q-40 0-68-28t-28-68v-192q0-40 28-68t68-28h192q40 0 68 28t28 68zm0-512v192q0 40-28 68t-68 28H800q-40 0-68-28t-28-68V736q0-40 28-68t68-28h192q40 0 68 28t28 68zm0-512v192q0 40-28 68t-68 28H800q-40 0-68-28t-28-68V224q0-40 28-68t68-28h192q40 0 68 28t28 68z"/></svg>
- if current_user - if current_user
- if note.emoji_awardable? - if note.emoji_awardable?
- user_authored = note.user_authored?(current_user) - user_authored = note.user_authored?(current_user)
.note-actions-item
= link_to '#', title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored} has-tooltip", data: { position: 'right' } do = link_to '#', title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored} has-tooltip", data: { position: 'right' } do
= icon('spinner spin') = icon('spinner spin')
%span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face') %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
%span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley') %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
%span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile') %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
- if note_editable
.note-actions-item
= button_tag title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip btn btn-transparent', data: { container: 'body' } do
%span.link-highlight
= custom_icon('icon_pencil')
= render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable
---
title: move edit comment button outside of dropdown
merge_request:
author:
...@@ -299,9 +299,6 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -299,9 +299,6 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
step 'I change the comment "Line is wrong" to "Typo, please fix" on diff' do step 'I change the comment "Line is wrong" to "Typo, please fix" on diff' do
page.within('.diff-file:nth-of-type(5) .note') do page.within('.diff-file:nth-of-type(5) .note') do
find('.more-actions').click
find('.more-actions .dropdown-menu li', match: :first)
find('.js-note-edit').click find('.js-note-edit').click
page.within('.current-note-edit-form', visible: true) do page.within('.current-note-edit-form', visible: true) do
......
...@@ -11,8 +11,8 @@ module SharedNote ...@@ -11,8 +11,8 @@ module SharedNote
note = find('.note') note = find('.note')
note.hover note.hover
note.find('.more-actions').click find('.more-actions').click
note.find('.more-actions .dropdown-menu li', match: :first) find('.more-actions .dropdown-menu li', match: :first)
find(".js-note-delete").click find(".js-note-delete").click
end end
...@@ -147,9 +147,6 @@ module SharedNote ...@@ -147,9 +147,6 @@ module SharedNote
note = find('.note') note = find('.note')
note.hover note.hover
note.find('.more-actions').click
note.find('.more-actions .dropdown-menu li', match: :first)
note.find('.js-note-edit').click note.find('.js-note-edit').click
end end
......
...@@ -133,8 +133,6 @@ feature 'Issue notes polling', :js do ...@@ -133,8 +133,6 @@ feature 'Issue notes polling', :js do
def click_edit_action(note) def click_edit_action(note)
note_element = find("#note_#{note.id}") note_element = find("#note_#{note.id}")
open_more_actions_dropdown(note)
note_element.find('.js-note-edit').click note_element.find('.js-note-edit').click
end end
end end
...@@ -157,7 +157,7 @@ feature 'Diff note avatars', js: true do ...@@ -157,7 +157,7 @@ feature 'Diff note avatars', js: true do
end end
page.within find("[id='#{position.line_code(project.repository)}']") do page.within find("[id='#{position.line_code(project.repository)}']") do
find('.diff-notes-collapse').click find('.diff-notes-collapse').trigger('click')
expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) expect(page).to have_selector('img.js-diff-comment-avatar', count: 3)
expect(find('.diff-comments-more-count')).to have_content '+1' expect(find('.diff-comments-more-count')).to have_content '+1'
......
...@@ -75,7 +75,6 @@ describe 'Merge requests > User posts notes', :js do ...@@ -75,7 +75,6 @@ describe 'Merge requests > User posts notes', :js do
describe 'editing the note' do describe 'editing the note' do
before do before do
find('.note').hover find('.note').hover
open_more_actions_dropdown(note)
find('.js-note-edit').click find('.js-note-edit').click
end end
...@@ -104,7 +103,6 @@ describe 'Merge requests > User posts notes', :js do ...@@ -104,7 +103,6 @@ describe 'Merge requests > User posts notes', :js do
wait_for_requests wait_for_requests
find('.note').hover find('.note').hover
open_more_actions_dropdown(note)
find('.js-note-edit').click find('.js-note-edit').click
...@@ -132,7 +130,6 @@ describe 'Merge requests > User posts notes', :js do ...@@ -132,7 +130,6 @@ describe 'Merge requests > User posts notes', :js do
describe 'deleting an attachment' do describe 'deleting an attachment' do
before do before do
find('.note').hover find('.note').hover
open_more_actions_dropdown(note)
find('.js-note-edit').click find('.js-note-edit').click
end end
......
...@@ -91,11 +91,7 @@ describe 'Comments on personal snippets', :js do ...@@ -91,11 +91,7 @@ describe 'Comments on personal snippets', :js do
context 'when editing a note' do context 'when editing a note' do
it 'changes the text' do it 'changes the text' do
open_more_actions_dropdown(snippet_notes[0]) find('.js-note-edit').click
page.within("#notes-list li#note_#{snippet_notes[0].id}") do
click_on 'Edit comment'
end
page.within('.current-note-edit-form') do page.within('.current-note-edit-form') do
fill_in 'note[note]', with: 'new content' fill_in 'note[note]', with: 'new content'
......
...@@ -7,15 +7,18 @@ shared_examples 'reportable note' do ...@@ -7,15 +7,18 @@ shared_examples 'reportable note' do
let(:more_actions_selector) { '.more-actions.dropdown' } let(:more_actions_selector) { '.more-actions.dropdown' }
let(:abuse_report_path) { new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) } let(:abuse_report_path) { new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) }
it 'has an edit button' do
expect(comment).to have_selector('.js-note-edit')
end
it 'has a `More actions` dropdown' do it 'has a `More actions` dropdown' do
expect(comment).to have_selector(more_actions_selector) expect(comment).to have_selector(more_actions_selector)
end end
it 'dropdown has Edit, Report and Delete links' do it 'dropdown has Report and Delete links' do
dropdown = comment.find(more_actions_selector) dropdown = comment.find(more_actions_selector)
open_dropdown(dropdown) open_dropdown(dropdown)
expect(dropdown).to have_button('Edit comment')
expect(dropdown).to have_link('Report as abuse', href: abuse_report_path) expect(dropdown).to have_link('Report as abuse', href: abuse_report_path)
expect(dropdown).to have_link('Delete comment', href: note_url(note, project)) expect(dropdown).to have_link('Delete comment', href: note_url(note, project))
end end
......
...@@ -24,18 +24,16 @@ describe 'projects/notes/_more_actions_dropdown' do ...@@ -24,18 +24,16 @@ describe 'projects/notes/_more_actions_dropdown' do
expect(rendered).not_to have_selector('.dropdown.more-actions') expect(rendered).not_to have_selector('.dropdown.more-actions')
end end
it 'shows Report as abuse, Edit and Delete buttons if editable and not current users comment' do it 'shows Report as abuse and Delete buttons if editable and not current users comment' do
render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: true, note: note render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: true, note: note
expect(rendered).to have_link('Report as abuse') expect(rendered).to have_link('Report as abuse')
expect(rendered).to have_button('Edit comment')
expect(rendered).to have_link('Delete comment') expect(rendered).to have_link('Delete comment')
end end
it 'shows Edit and Delete buttons if editable and current users comment' do it 'shows Delete button if editable and current users comment' do
render 'projects/notes/more_actions_dropdown', current_user: author_user, note_editable: true, note: note render 'projects/notes/more_actions_dropdown', current_user: author_user, note_editable: true, note: note
expect(rendered).to have_button('Edit comment')
expect(rendered).to have_link('Delete comment') expect(rendered).to have_link('Delete comment')
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment