Rearrange discussion resolve buttons & update button display

- next-discuss button is always show unless there is only 1 discussion
- regroup buttons arrangements
This commit is contained in:
Samantha Ming 2019-06-28 07:32:03 +00:00 committed by Phil Hughes
parent ae68c7ea14
commit 4bf0434359
7 changed files with 54 additions and 22 deletions

View File

@ -39,20 +39,27 @@ export default {
</script>
<template>
<div class="discussion-with-resolve-btn">
<div class="discussion-with-resolve-btn clearfix">
<reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" />
<resolve-discussion-button
v-if="discussion.resolvable"
:is-resolving="isResolving"
:button-title="resolveButtonTitle"
@onClick="$emit('resolve')"
/>
<div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group">
<resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" />
<jump-to-next-discussion-button
v-if="shouldShowJumpToNextDiscussion"
@onClick="$emit('jumpToNextDiscussion')"
<div class="btn-group discussion-actions" role="group">
<resolve-discussion-button
v-if="discussion.resolvable"
:is-resolving="isResolving"
:button-title="resolveButtonTitle"
@onClick="$emit('resolve')"
/>
<resolve-with-issue-button
v-if="discussion.resolvable && resolveWithIssuePath"
:url="resolveWithIssuePath"
/>
</div>
<div
v-if="discussion.resolvable && shouldShowJumpToNextDiscussion"
class="btn-group discussion-actions ml-sm-2"
>
<jump-to-next-discussion-button @onClick="$emit('jumpToNextDiscussion')" />
</div>
</div>
</template>

View File

@ -126,10 +126,7 @@ export default {
return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved');
},
shouldShowJumpToNextDiscussion() {
return this.showJumpToNextDiscussion(
this.discussion.id,
this.discussionsByDiffOrder ? 'diff' : 'discussion',
);
return this.showJumpToNextDiscussion(this.discussionsByDiffOrder ? 'diff' : 'discussion');
},
shouldRenderDiffs() {
return this.discussion.diff_discussion && this.renderDiffFile;

View File

@ -61,15 +61,13 @@ export const unresolvedDiscussionsCount = state => state.unresolvedDiscussionsCo
export const resolvableDiscussionsCount = state => state.resolvableDiscussionsCount;
export const hasUnresolvedDiscussions = state => state.hasUnresolvedDiscussions;
export const showJumpToNextDiscussion = (state, getters) => (discussionId, mode = 'discussion') => {
export const showJumpToNextDiscussion = (state, getters) => (mode = 'discussion') => {
const orderedDiffs =
mode !== 'discussion'
? getters.unresolvedDiscussionsIdsByDiff
: getters.unresolvedDiscussionsIdsByDate;
const indexOf = orderedDiffs.indexOf(discussionId);
return indexOf !== -1 && indexOf < orderedDiffs.length - 1;
return orderedDiffs.length > 1;
};
export const isDiscussionResolved = (state, getters) => discussionId =>

View File

@ -657,6 +657,10 @@ $note-form-margin-left: 72px;
margin-left: -1px;
}
.btn-group > .discussion-create-issue-btn {
margin-left: -2px;
}
svg {
height: 15px;
}

View File

@ -0,0 +1,6 @@
---
title: Improve discussion reply buttons layout and how jump to next discussion button
appears
merge_request: 29779
author:
type: changed

View File

@ -362,14 +362,14 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
end
end
it 'shows jump to next discussion button except on last discussion' do
it 'shows jump to next discussion button on all discussions' do
wait_for_requests
all_discussion_replies = page.all('.discussion-reply-holder')
expect(all_discussion_replies.count).to eq(2)
expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1)
expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(0)
expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1)
end
it 'displays next discussion even if hidden' do

View File

@ -32,6 +32,26 @@ describe('Getters Notes Store', () => {
};
});
describe('showJumpToNextDiscussion', () => {
it('should return true if there are 2 or more unresolved discussions', () => {
const localGetters = {
unresolvedDiscussionsIdsByDate: ['123', '456'],
allResolvableDiscussions: [],
};
expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(true);
});
it('should return false if there are 1 or less unresolved discussions', () => {
const localGetters = {
unresolvedDiscussionsIdsByDate: ['123'],
allResolvableDiscussions: [],
};
expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(false);
});
});
describe('discussions', () => {
it('should return all discussions in the store', () => {
expect(getters.discussions(state)).toEqual([individualNote]);