Linking to a comment in a resolved non-diff discussion in an MR doesn't expand the discussion
When opening a direct link to a comment of a resolved non-diff discussion in an MR, the discussion isn't expanded to show the comment being linked to.
This may be due to our redesigned non-diff discussions where the first comment is shown and the rest is collapsed.
Findings
In NoteableDiscussion
, we have isRepliesCollapsed
which determines whether a discussion is expanded or not. This is a local property of the component with a default value based on whether the discussion is resolved or not.
This means it doesn't get affected when checkLocationHash()
mutates discussion.expanded
when the discussion is being linked to.
I also noticed that this component also has shouldShowDiscussions
which depends on discussion.expanded
but an OR condition was added to make it true all of the time.
In the template, this is also used like so: <div v-if="shouldShowDiscussions" class="discussion-body">
. We don't ever want this to be false
because the discussion would just look like a gray line.
Possible solution
-
Remove
v-if="shouldShowDiscussions"
indiv.discussion-body
. -
Change
shouldShowDiscussions
to something like:diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index fc51998935d..44b0fe4d772 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -176,10 +176,7 @@ export default { return ''; }, shouldShowDiscussions() { - const { expanded, resolved } = this.discussion; - const isResolvedNonDiffDiscussion = !this.discussion.diff_discussion && resolved; - - return expanded || this.alwaysExpanded || isResolvedNonDiffDiscussion; + return this.discussion.expanded || this.alwaysExpanded; }, actionText() { const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`;
-
Remove
isRepliesCollapsed
and change all references to it in the template to!shouldShowDiscussions
-
Remove
toggleReplies
and use the already existingtoggleDiscussionHandler
instead. This actually triggers theTOGGLE_DISCUSSION
mutation instead of just toggling the internalisRepliesCollapsed
boolean.
It looks like this.discussion.expanded
is being set properly by the backend from DiscussionEntity
so we should just base the initial state on that instead of having special logic here in the view.