Colour-code PR Code Comments tab indicator by unresolved state #2958
Service Desk opened 2 days ago

Follow-up from issue #2957 (design locked).

Enhance the pull request Code Comments tab indicator so colour and fill reflect unresolved comments vs unviewed activity.

Locked design

Indicator matrix

Unresolved comments on the PR?Unviewed code-comment activity?Indicator
YesYesFilled warning/orange (var(--warning))
YesNoUnfilled (outline) warning/orange
NoYesFilled primary/blue (var(--primary))
NoNoNone

Other decisions

  1. Visit semantics: Keep existing PR tab-level visit tracking (isCodeCommentsVisitedAfter / visitPullRequestCodeComments). Opening the Code Comments tab clears the “unviewed” (filled) state; outline remains while any unresolved comments exist.
  2. Open definition: Any unresolved code comment on the PR (not limited to whether the latest activity touched an open thread).
  3. Tab filter: Do not auto-filter to unresolved when the open-coloured indicator is shown.
  4. Tooltip: None (no tooltip exists today; do not add one).
  5. Unfilled style: Same-size outline circle in warning colour, transparent fill (stroke/edge only; same footprint as today’s filled dot).

Current behaviour (code anchors)

  • Tab link + SVG dot: PullRequestDetailPage.html (codeCommentsTabLinkFrag)
  • Class toggle: PullRequestDetailPage.java — adds new when getCodeCommentsUpdateDate() is after the user’s code-comments visit
  • Styles: pull-request-detail.cssul>li>a.code-comments.new .icon fills with var(--primary) and unhides the icon
  • Visit update: PullRequestCodeCommentsPage calls VisitInfoService.visitPullRequestCodeComments

Implementation plan

  1. Detect unresolved comments on the PR
    Add a cheap helper (e.g. on PullRequest or inline in the tab model) that returns whether any linked CodeComment has !isResolved(). Prefer a path that does not add N+1 queries on every PR detail render (reuse already-loaded comments if present, or a targeted exists/count query / denormalized flag if loading the full collection is too heavy). Ensure the ChangeObserver on the tab still refreshes when comment status changes.

  2. Expand tab CSS classes
    Replace the single new flag with a small class matrix, for example:

    • new — unviewed activity (filled)
    • unresolved — at least one unresolved comment (warning colour; filled or outline depending on new)
    • Logic in the existing AttributeAppender / LoadableDetachableModel on the Code Comments tab link
  3. CSS for filled vs outline
    In pull-request-detail.css:

    • Filled blue: keep current .code-comments.new .icon with fill: var(--primary) when not unresolved
    • Filled warning: .code-comments.new.unresolved .iconfill: var(--warning)
    • Outline warning: .code-comments.unresolved:not(.new) .icon → transparent/fill: none, stroke: var(--warning), stroke width tuned so the ring matches the 12×12 footprint of the filled dot
      Reuse the existing dot SVG if stroke styling works; otherwise add a dedicated outline-dot symbol of the same size.
  4. Do not change default filter
    Leave PullRequestCodeCommentsPage default listing as today (all comments; no auto unresolved-only).

  5. No tooltip
    Do not add title / tip content on the indicator.

Acceptance

  • Unresolved + unviewed → filled orange dot
  • Unresolved + viewed (no newer activity) → outline orange dot
  • All resolved + unviewed → filled blue dot
  • All resolved + viewed → no dot
  • Visiting Code Comments clears filled/unviewed; outline remains if unresolved comments still exist
  • Resolving the last open comment removes the outline
  • Default comments filter unchanged; no tooltip
1/1
Type
Improvement
Priority
Normal
Assignee
Labels
No labels
Issue Votes (0)
Watchers (2)
Reference
OD-2958
Please wait...
Connection lost or session expired, reload to recover
Page is in error, reload to recover