Commit 1ba7df0d authored by Mike Greiling's avatar Mike Greiling

Replace styled reply button with textarea

In discussion threads there is a "Reply..." button styled to look like
a textarea which, when clicked, will open up a full GFM discussion
form with a textarea element. This changes this button into an actual
textarea to improve accessibility.
parent 6fed8970
...@@ -257,8 +257,8 @@ export default { ...@@ -257,8 +257,8 @@ export default {
<reply-placeholder <reply-placeholder
v-if="!isFormVisible" v-if="!isFormVisible"
class="qa-discussion-reply" class="qa-discussion-reply"
:button-text="__('Reply...')" :placeholder-text="__('Reply...')"
@onClick="showForm" @focus="showForm"
/> />
<apollo-mutation <apollo-mutation
v-else v-else
......
...@@ -35,8 +35,8 @@ export default { ...@@ -35,8 +35,8 @@ export default {
<slot v-if="hasForm" name="form"></slot> <slot v-if="hasForm" name="form"></slot>
<template v-else-if="renderReplyPlaceholder"> <template v-else-if="renderReplyPlaceholder">
<reply-placeholder <reply-placeholder
:button-text="__('Start a new discussion...')" :placeholder-text="__('Start a new discussion...')"
@onClick="$emit('showNewDiscussionForm')" @focus="$emit('showNewDiscussionForm')"
/> />
</template> </template>
</template> </template>
......
...@@ -50,8 +50,8 @@ export default { ...@@ -50,8 +50,8 @@ export default {
<div class="discussion-with-resolve-btn clearfix"> <div class="discussion-with-resolve-btn clearfix">
<reply-placeholder <reply-placeholder
data-qa-selector="discussion_reply_tab" data-qa-selector="discussion_reply_tab"
:button-text="s__('MergeRequests|Reply...')" :placeholder-text="s__('MergeRequests|Reply...')"
@onClick="$emit('showReplyForm')" @focus="$emit('showReplyForm')"
/> />
<div v-if="userCanResolveDiscussion" class="btn-group discussion-actions" role="group"> <div v-if="userCanResolveDiscussion" class="btn-group discussion-actions" role="group">
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
export default { export default {
name: 'ReplyPlaceholder', name: 'ReplyPlaceholder',
props: { props: {
buttonText: { placeholderText: {
type: String, type: String,
required: true, required: true,
}, },
...@@ -11,13 +11,11 @@ export default { ...@@ -11,13 +11,11 @@ export default {
</script> </script>
<template> <template>
<button <textarea
ref="button" rows="1"
type="button" class="reply-placeholder-text-field js-vue-discussion-reply"
class="js-vue-discussion-reply btn btn-text-field" :placeholder="placeholderText"
:title="s__('MergeRequests|Add a reply')" :title="s__('MergeRequests|Add a reply')"
@click="$emit('onClick')" @focus="$emit('focus')"
> ></textarea>
{{ buttonText }}
</button>
</template> </template>
...@@ -216,6 +216,7 @@ ...@@ -216,6 +216,7 @@
color: $gray-700; color: $gray-700;
} }
// deprecated class
&.btn-text-field { &.btn-text-field {
width: 100%; width: 100%;
text-align: left; text-align: left;
......
...@@ -303,7 +303,15 @@ table { ...@@ -303,7 +303,15 @@ table {
} }
} }
.btn-text-field { .reply-placeholder-text-field {
border-radius: $gl-border-radius-base;
width: 100%;
resize: none;
padding: $gl-padding-4 $gl-padding;
border-color: $border-color;
color: $gray-darkest;
background-color: $white;
@include media-breakpoint-down(xs) { @include media-breakpoint-down(xs) {
margin-bottom: $gl-padding-8; margin-bottom: $gl-padding-8;
} }
......
...@@ -1008,7 +1008,7 @@ RSpec.describe 'GFM autocomplete', :js do ...@@ -1008,7 +1008,7 @@ RSpec.describe 'GFM autocomplete', :js do
end end
def start_and_cancel_discussion def start_and_cancel_discussion
click_button('Reply...') find('.js-vue-discussion-reply').click
fill_in('note_note', with: 'Whoops!') fill_in('note_note', with: 'Whoops!')
......
...@@ -223,7 +223,7 @@ end ...@@ -223,7 +223,7 @@ end
def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false) def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false)
page.within(first('.diff-files-holder .discussion-reply-holder')) do page.within(first('.diff-files-holder .discussion-reply-holder')) do
click_button('Reply...') first('.js-vue-discussion-reply').click
fill_in('note_note', with: text) fill_in('note_note', with: text)
......
...@@ -192,7 +192,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do ...@@ -192,7 +192,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
it 'adds as discussion' do it 'adds as discussion' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false) should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false)
expect(page).to have_css('.notes_holder .note.note-discussion', count: 1) expect(page).to have_css('.notes_holder .note.note-discussion', count: 1)
expect(page).to have_button('Reply...') expect(page).to have_css('.js-vue-discussion-reply')
end end
end end
end end
......
...@@ -149,7 +149,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -149,7 +149,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
it 'allows user to comment' do it 'allows user to comment' do
page.within '.diff-content' do page.within '.diff-content' do
click_button 'Reply...' find('.js-vue-discussion-reply').click
find(".js-unresolve-checkbox").set false find(".js-unresolve-checkbox").set false
find('.js-note-text').set 'testing' find('.js-note-text').set 'testing'
...@@ -179,7 +179,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -179,7 +179,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
it 'allows user to comment & unresolve thread' do it 'allows user to comment & unresolve thread' do
page.within '.diff-content' do page.within '.diff-content' do
click_button 'Reply...' find('.js-vue-discussion-reply').click
find('.js-note-text').set 'testing' find('.js-note-text').set 'testing'
...@@ -208,7 +208,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -208,7 +208,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
it 'allows user to comment & resolve thread' do it 'allows user to comment & resolve thread' do
page.within '.diff-content' do page.within '.diff-content' do
click_button 'Reply...' find('.js-vue-discussion-reply').click
find('.js-note-text').set 'testing' find('.js-note-text').set 'testing'
...@@ -442,7 +442,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -442,7 +442,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
it 'allows user to comment & resolve thread' do it 'allows user to comment & resolve thread' do
page.within '.diff-content' do page.within '.diff-content' do
click_button 'Reply...' find('.js-vue-discussion-reply').click
find('.js-note-text').set 'testing' find('.js-note-text').set 'testing'
...@@ -461,7 +461,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -461,7 +461,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
page.within '.diff-content' do page.within '.diff-content' do
click_button 'Resolve thread' click_button 'Resolve thread'
click_button 'Reply...' find('.js-vue-discussion-reply').click
find('.js-note-text').set 'testing' find('.js-note-text').set 'testing'
......
...@@ -37,7 +37,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do ...@@ -37,7 +37,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
end end
it 'does not render avatars after commenting on discussion tab' do it 'does not render avatars after commenting on discussion tab' do
click_button 'Reply...' find('.js-vue-discussion-reply').click
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
find('.note-textarea').native.send_keys('Test comment') find('.note-textarea').native.send_keys('Test comment')
...@@ -132,7 +132,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do ...@@ -132,7 +132,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
end end
it 'adds avatar when commenting' do it 'adds avatar when commenting' do
click_button 'Reply...' first('.js-vue-discussion-reply').click
page.within '.js-discussion-note-form' do page.within '.js-discussion-note-form' do
find('.js-note-text').native.send_keys('Test') find('.js-note-text').native.send_keys('Test')
...@@ -151,7 +151,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do ...@@ -151,7 +151,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
it 'adds multiple comments' do it 'adds multiple comments' do
3.times do 3.times do
click_button 'Reply...' first('.js-vue-discussion-reply').click
page.within '.js-discussion-note-form' do page.within '.js-discussion-note-form' do
find('.js-note-text').native.send_keys('Test') find('.js-note-text').native.send_keys('Test')
......
...@@ -60,7 +60,7 @@ RSpec.describe 'Merge request > User sees threads', :js do ...@@ -60,7 +60,7 @@ RSpec.describe 'Merge request > User sees threads', :js do
it 'can be replied to' do it 'can be replied to' do
within(".discussion[data-discussion-id='#{discussion_id}']") do within(".discussion[data-discussion-id='#{discussion_id}']") do
click_button 'Reply...' find('.js-vue-discussion-reply').click
fill_in 'note[note]', with: 'Test!' fill_in 'note[note]', with: 'Test!'
click_button 'Comment' click_button 'Comment'
......
...@@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User sees notes from forked project', :js do ...@@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User sees notes from forked project', :js do
expect(page).to have_content('A commit comment') expect(page).to have_content('A commit comment')
page.within('.discussion-notes') do page.within('.discussion-notes') do
find('.btn-text-field').click find('.js-vue-discussion-reply').click
scroll_to(page.find('#note_note', visible: false)) scroll_to(page.find('#note_note', visible: false))
find('#note_note').send_keys('A reply comment') find('#note_note').send_keys('A reply comment')
find('.js-comment-button').click find('.js-comment-button').click
......
...@@ -83,7 +83,7 @@ RSpec.describe 'User comments on a diff', :js do ...@@ -83,7 +83,7 @@ RSpec.describe 'User comments on a diff', :js do
wait_for_requests wait_for_requests
click_button 'Reply...' first('.js-vue-discussion-reply').click
find('.js-suggestion-btn').click find('.js-suggestion-btn').click
......
...@@ -93,7 +93,7 @@ describe('Design discussions component', () => { ...@@ -93,7 +93,7 @@ describe('Design discussions component', () => {
}); });
it('does not render a checkbox in reply form', () => { it('does not render a checkbox in reply form', () => {
findReplyPlaceholder().vm.$emit('onClick'); findReplyPlaceholder().vm.$emit('focus');
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(findResolveCheckbox().exists()).toBe(false); expect(findResolveCheckbox().exists()).toBe(false);
...@@ -124,7 +124,7 @@ describe('Design discussions component', () => { ...@@ -124,7 +124,7 @@ describe('Design discussions component', () => {
}); });
it('renders a checkbox with Resolve thread text in reply form', () => { it('renders a checkbox with Resolve thread text in reply form', () => {
findReplyPlaceholder().vm.$emit('onClick'); findReplyPlaceholder().vm.$emit('focus');
wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
...@@ -193,7 +193,7 @@ describe('Design discussions component', () => { ...@@ -193,7 +193,7 @@ describe('Design discussions component', () => {
}); });
it('renders a checkbox with Unresolve thread text in reply form', () => { it('renders a checkbox with Unresolve thread text in reply form', () => {
findReplyPlaceholder().vm.$emit('onClick'); findReplyPlaceholder().vm.$emit('focus');
wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
...@@ -205,7 +205,7 @@ describe('Design discussions component', () => { ...@@ -205,7 +205,7 @@ describe('Design discussions component', () => {
it('hides reply placeholder and opens form on placeholder click', () => { it('hides reply placeholder and opens form on placeholder click', () => {
createComponent(); createComponent();
findReplyPlaceholder().vm.$emit('onClick'); findReplyPlaceholder().vm.$emit('focus');
wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id }); wrapper.setProps({ discussionWithOpenForm: defaultMockDiscussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
...@@ -307,7 +307,7 @@ describe('Design discussions component', () => { ...@@ -307,7 +307,7 @@ describe('Design discussions component', () => {
it('emits openForm event on opening the form', () => { it('emits openForm event on opening the form', () => {
createComponent(); createComponent();
findReplyPlaceholder().vm.$emit('onClick'); findReplyPlaceholder().vm.$emit('focus');
expect(wrapper.emitted('open-form')).toBeTruthy(); expect(wrapper.emitted('open-form')).toBeTruthy();
}); });
......
...@@ -96,7 +96,7 @@ describe('DiscussionActions', () => { ...@@ -96,7 +96,7 @@ describe('DiscussionActions', () => {
it('emits showReplyForm event when clicking on reply placeholder', () => { it('emits showReplyForm event when clicking on reply placeholder', () => {
jest.spyOn(wrapper.vm, '$emit'); jest.spyOn(wrapper.vm, '$emit');
wrapper.find(ReplyPlaceholder).find('button').trigger('click'); wrapper.find(ReplyPlaceholder).find('textarea').trigger('focus');
expect(wrapper.vm.$emit).toHaveBeenCalledWith('showReplyForm'); expect(wrapper.vm.$emit).toHaveBeenCalledWith('showReplyForm');
}); });
......
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
const buttonText = 'Test Button Text'; const placeholderText = 'Test Button Text';
describe('ReplyPlaceholder', () => { describe('ReplyPlaceholder', () => {
let wrapper; let wrapper;
const findButton = () => wrapper.find({ ref: 'button' }); const findTextarea = () => wrapper.find('.js-vue-discussion-reply');
beforeEach(() => { beforeEach(() => {
wrapper = shallowMount(ReplyPlaceholder, { wrapper = shallowMount(ReplyPlaceholder, {
propsData: { propsData: {
buttonText, placeholderText,
}, },
}); });
}); });
...@@ -20,17 +20,17 @@ describe('ReplyPlaceholder', () => { ...@@ -20,17 +20,17 @@ describe('ReplyPlaceholder', () => {
wrapper.destroy(); wrapper.destroy();
}); });
it('emits onClick event on button click', () => { it('emits focus event on button click', () => {
findButton().trigger('click'); findTextarea().trigger('focus');
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(wrapper.emitted()).toEqual({ expect(wrapper.emitted()).toEqual({
onClick: [[]], focus: [[]],
}); });
}); });
}); });
it('should render reply button', () => { it('should render reply button', () => {
expect(findButton().text()).toEqual(buttonText); expect(findTextarea().element.placeholder).toEqual(placeholderText);
}); });
}); });
...@@ -65,7 +65,7 @@ describe('noteable_discussion component', () => { ...@@ -65,7 +65,7 @@ describe('noteable_discussion component', () => {
expect(wrapper.vm.isReplying).toEqual(false); expect(wrapper.vm.isReplying).toEqual(false);
const replyPlaceholder = wrapper.find(ReplyPlaceholder); const replyPlaceholder = wrapper.find(ReplyPlaceholder);
replyPlaceholder.vm.$emit('onClick'); replyPlaceholder.vm.$emit('focus');
await nextTick(); await nextTick();
expect(wrapper.vm.isReplying).toEqual(true); expect(wrapper.vm.isReplying).toEqual(true);
......
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