diff options
author | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-03-22 22:33:01 +0800 |
---|---|---|
committer | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-03-26 09:51:47 +0800 |
commit | 7495140de2b4412240a09d7ee72800a84f57433e (patch) | |
tree | b557857c0aa33663f043de47b3d059afb7941a1d | |
parent | 66054aeb13315ccf99f167081d09b7be75be3e46 (diff) | |
download | gitlab-ce-7495140de2b4412240a09d7ee72800a84f57433e.tar.gz |
Expand discussion when opening link to comment
Makes discussion expansion depend on `discussion.expanded`
3 files changed, 44 insertions, 25 deletions
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 0fabbfb06b5..a3d664a738f 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -87,13 +87,10 @@ export default { }, }, data() { - const { diff_discussion: isDiffDiscussion, resolved } = this.discussion; - return { isReplying: false, isResolving: false, resolveAsThread: true, - isRepliesCollapsed: Boolean(!isDiffDiscussion && resolved), }; }, computed: { @@ -178,11 +175,11 @@ export default { return ''; }, - shouldShowDiscussions() { - const { expanded, resolved } = this.discussion; - const isResolvedNonDiffDiscussion = !this.discussion.diff_discussion && resolved; - - return expanded || this.alwaysExpanded || isResolvedNonDiffDiscussion; + isExpanded() { + return this.discussion.expanded || this.alwaysExpanded; + }, + shouldHideDiscussionBody() { + return this.shouldRenderDiffs && !this.isExpanded; }, actionText() { const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`; @@ -282,9 +279,6 @@ export default { toggleDiscussionHandler() { this.toggleDiscussion({ discussionId: this.discussion.id }); }, - toggleReplies() { - this.isRepliesCollapsed = !this.isRepliesCollapsed; - }, showReplyForm() { this.isReplying = true; }, @@ -405,7 +399,7 @@ Please check your network connection and try again.`; /> </div> </div> - <div v-if="shouldShowDiscussions" class="discussion-body"> + <div v-if="!shouldHideDiscussionBody" class="discussion-body"> <component :is="wrapperComponent" v-bind="wrapperComponentProps" @@ -436,11 +430,11 @@ Please check your network connection and try again.`; </component> <toggle-replies-widget v-if="hasReplies" - :collapsed="isRepliesCollapsed" + :collapsed="!isExpanded" :replies="replies" - @toggle="toggleReplies" + @toggle="toggleDiscussionHandler" /> - <template v-if="!isRepliesCollapsed"> + <template v-if="isExpanded"> <component :is="componentName(note)" v-for="note in replies" @@ -467,7 +461,7 @@ Please check your network connection and try again.`; </template> </ul> <div - v-if="!isRepliesCollapsed || !hasReplies" + v-if="isExpanded || !hasReplies" :class="{ 'is-replying': isReplying }" class="discussion-reply-holder" > diff --git a/changelogs/unreleased/59352-fix-mr-discussion-expansion.yml b/changelogs/unreleased/59352-fix-mr-discussion-expansion.yml new file mode 100644 index 00000000000..ab9ad53835c --- /dev/null +++ b/changelogs/unreleased/59352-fix-mr-discussion-expansion.yml @@ -0,0 +1,5 @@ +--- +title: Expand resolved discussion when linking to a comment in the discussion +merge_request: 26483 +author: +type: fixed diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb index 8c2599615cb..2f7d359575e 100644 --- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb +++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb @@ -5,9 +5,7 @@ describe 'Merge request > User scrolls to note on load', :js do let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, author: user) } let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } - let(:resolved_note) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) } let(:fragment_id) { "#note_#{note.id}" } - let(:collapsed_fragment_id) { "#note_#{resolved_note.id}" } before do sign_in(user) @@ -45,13 +43,35 @@ describe 'Merge request > User scrolls to note on load', :js do end end - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'expands collapsed notes' do - visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}" - note_element = find(collapsed_fragment_id) - note_container = note_element.ancestor('.timeline-content') + context 'resolved notes' do + let(:collapsed_fragment_id) { "#note_#{resolved_note.id}" } - expect(note_element.visible?).to eq true - expect(note_container.find('.line_content.noteable_line.old', match: :first).visible?).to eq true + context 'when diff note' do + let(:resolved_note) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + + it 'expands collapsed notes' do + visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}" + + note_element = find(collapsed_fragment_id) + diff_container = note_element.ancestor('.diff-content') + + expect(note_element.visible?).to eq(true) + expect(diff_container.visible?).to eq(true) + end + end + + context 'when non-diff note' do + let(:non_diff_discussion) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + let(:resolved_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project, in_reply_to: non_diff_discussion) } + + it 'expands collapsed replies' do + visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}" + + note_element = find(collapsed_fragment_id) + + expect(note_element.visible?).to eq(true) + expect(note_element.sibling('.replies-toggle')[:class]).to include('expanded') + end + end end end |