feat(pulls): support per-commit review (#39326)

Opening a single commit of a PR let you look, but not review. The Review
button was disabled and there was no way to reach the next or previous
commit. This finishes https://github.com/go-gitea/gitea/pull/37475,
which only reached the head commit.

- Reviews can be submitted from a single commit, a commit range stays
disabled
- A bar above a single-commit diff shows the commit, its position in the
PR and prev/next buttons
- The commit dropdown marks the commits on screen
- Timeline commits open the PR's commit view, commits force-pushed out
of the PR redirect to the repository commit page

Assisted-by: Claude Code (Fable 5.1)
Co-authored-by: bircni <bircni@icloud.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>

options/locale/locale_en-US.json

@@ -1791,10 +1791,12 @@
   "repo.pulls.filter_branch": "Filter branch",
   "repo.pulls.show_all_commits": "Show all commits",
   "repo.pulls.show_changes_since_your_last_review": "Show changes since your last review",
-  "repo.pulls.showing_only_single_commit": "Showing only changes of commit %[1]s",
   "repo.pulls.showing_specified_commit_range": "Showing only changes between %[1]s..%[2]s",
   "repo.pulls.select_commit_hold_shift_for_range": "Select commit. Hold Shift and click to select a range.",
-  "repo.pulls.review_only_possible_for_full_diff": "Review is only possible when viewing the full diff",
+  "repo.pulls.review_not_possible_for_commit_range": "Review is not possible when viewing a commit range",
+  "repo.pulls.commit_position": "Commit %[1]d of %[2]d",
+  "repo.pulls.previous_commit": "Previous commit",
+  "repo.pulls.next_commit": "Next commit",
   "repo.pulls.filter_changes_by_commit": "Filter by commit",
   "repo.pulls.nothing_to_compare": "There are no differences to show. There is no need to create a pull request.",
   "repo.pulls.no_common_history": "These branches do not share a common merge base. Select a different base or compare branch.",
@@ -2626,7 +2628,7 @@
   "repo.diff.comment.start_review": "Start review",
   "repo.diff.comment.reply": "Reply",
   "repo.diff.review": "Review",
-  "repo.diff.review.header": "Submit review",
+  "repo.diff.review.header": "Finish your review",
   "repo.diff.review.placeholder": "Review comment",
   "repo.diff.review.comment": "Comment",
   "repo.diff.review.approve": "Approve",

routers/web/repo/pull.go

@@ -6,11 +6,13 @@
 package repo
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"html"
 	"html/template"
 	"net/http"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -676,6 +678,26 @@ func indexCommit(commits []*git.Commit, commitID string) *git.Commit {
 	return nil
 }
 
+// setSingleCommitNavContext exposes the reviewed commit and its neighbors.
+func setSingleCommitNavContext(ctx *context.Context, commits []*git.Commit, commit *git.Commit) {
+	idx := slices.IndexFunc(commits, func(c *git.Commit) bool { return bytes.Equal(c.ID.RawValue(), commit.ID.RawValue()) })
+	if idx < 0 {
+		return
+	}
+
+	ctx.Data["SingleCommit"] = commit
+	ctx.Data["SingleCommitIndex"] = len(commits) - idx
+	ctx.Data["SingleCommitCount"] = len(commits)
+
+	// The commit list is newest-first while a reviewer walks it oldest-first, so the indexes are mirrored.
+	if idx+1 < len(commits) {
+		ctx.Data["SingleCommitPrevID"] = commits[idx+1].ID.String()
+	}
+	if idx > 0 {
+		ctx.Data["SingleCommitNextID"] = commits[idx-1].ID.String()
+	}
+}
+
 // ViewPullFiles render pull request changed files list page
 func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
 	var err error
@@ -703,11 +725,17 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
 	}
 
 	headCommitID := prCompareInfo.HeadCommitID
-	isSingleCommit := beforeCommitID == "" && afterCommitID != ""
-	// FIXME: when afterCommitID==headCommitID, isSingleCommit and isShowAllCommits can be both true, which doesn't seem right
-	isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prCompareInfo.CompareBase) && (afterCommitID == "" || afterCommitID == headCommitID)
+	// show all commits when:
+	// * both before and after commit IDs are empty (default view)
+	// * before commit ID is the PR's base commit and after commit ID is the PR's head commit (default view)
+	// * there is only one commit in the PR and after commit ID is the PR's head commit (single-commit view)
+	isShowAllCommits := (beforeCommitID == "" && afterCommitID == "") ||
+		(beforeCommitID == prCompareInfo.CompareBase && afterCommitID == headCommitID) ||
+		(beforeCommitID == "" && afterCommitID == headCommitID && len(prCompareInfo.Commits) == 1)
+	isSingleCommit := beforeCommitID == "" && afterCommitID != "" && !isShowAllCommits
 
-	ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit
+	// it's questionable why single commit review can be submitted, but the range commits review cannot be submitted
+	ctx.Data["CanSubmitReview"] = isShowAllCommits || isSingleCommit
 	ctx.Data["IsShowingAllCommits"] = isShowAllCommits
 
 	// "commits list" is half-open, half-closed: (base, head]
@@ -723,6 +751,10 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
 			return
 		}
 	}
+	if afterCommit == nil && isSingleCommit {
+		ctx.Redirect(ctx.Repo.RepoLink + "/commit/" + afterCommitID) // timeline commits outlive force pushes
+		return
+	}
 	if afterCommit == nil {
 		ctx.NotFound(nil)
 		return
@@ -756,6 +788,11 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
 	ctx.Data["CompareInfo"] = prCompareInfo
 	ctx.Data["AfterCommitID"] = afterCommitID
 	ctx.Data["BeforeCommitID"] = beforeCommitID
+	ctx.Data["ReviewCommitID"] = afterCommitID // the review happens on the "after commit" (the latest commit in the range)
+
+	if isSingleCommit {
+		setSingleCommitNavContext(ctx, prCompareInfo.Commits, afterCommit)
+	}
 
 	maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
 	files := ctx.FormStrings("files")

routers/web/repo/pull_review.go

@@ -56,6 +56,7 @@ func RenderNewCodeCommentForm(ctx *context.Context) {
 		return
 	}
 	ctx.Data["AfterCommitID"] = pullHeadCommitID
+	ctx.Data["ReviewCommitID"] = pullHeadCommitID // it's questionable why it only needs the head commit ID but not correct "AfterCommitID" (single commit review)
 	upload.AddUploadContext(ctx, "comment")
 	ctx.HTML(http.StatusOK, tplNewComment)
 }
@@ -209,6 +210,7 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
 		return
 	}
 	ctx.Data["AfterCommitID"] = pullHeadCommitID
+	ctx.Data["ReviewCommitID"] = pullHeadCommitID // it's questionable why it only needs the head commit ID but not correct "AfterCommitID" (single commit review)
 	ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
 		return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
 	}

templates/repo/commit_statuses.tmpl

@@ -1,6 +1,6 @@
 {{if .Statuses}}
 	{{if and (eq (len .Statuses) 1) .Status.TargetURL}}
-		<a class="flex-text-inline tw-no-underline {{.AdditionalClasses}}" data-global-init="initCommitStatuses" href="{{.Status.TargetURL}}">
+		<a class="flex-text-inline silenced {{.AdditionalClasses}}" data-global-init="initCommitStatuses" href="{{.Status.TargetURL}}">
 			{{template "repo/icons/commit_status" .Status}}
 		</a>
 	{{else}}

templates/repo/commits_list_small.tmpl

@@ -8,7 +8,7 @@
 		<span class="badge badge-commit">{{svg "octicon-git-commit"}}</span>
 		{{ctx.RenderUtils.AvatarStack $commit.AvatarStackData}}
 
-		{{$commitBaseLink := printf "%s/commit" $.comment.Issue.PullRequest.BaseRepo.Link}}
+		{{$commitBaseLink := printf "%s/commits" $.comment.Issue.Link}}{{/* "/owner/repo/pulls/{idx}/commits" */}}
 		{{$commitLink:= printf "%s/%s" $commitBaseLink (PathEscape $gitCommit.ID.String)}}
 
 		<span class="tw-flex-1 tw-font-mono gt-ellipsis" title="{{$gitCommit.MessageTitle}}">

templates/repo/diff/box.tmpl

@@ -36,6 +36,7 @@
 			{{template "repo/diff/options_dropdown" .}}
 			{{if .PageIsPullFiles}}
 				<div id="diff-commit-select" data-global-init="initDiffCommitSelect" data-merge-base="{{$.CompareInfo.CompareBase}}" data-issuelink="{{$.Issue.Link}}"
+					{{if not $.IsShowingAllCommits}}data-before-commit="{{$.BeforeCommitID}}" data-after-commit="{{$.AfterCommitID}}"{{end}}
 					data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}"
 					data-text-filter-changes-by-commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}">
 					{{/* the following will be replaced by vue component, but this avoids any loading artifacts till the vue component is initialized */}}
@@ -50,10 +51,8 @@
 		</div>
 	</div>
 	{{if not .DiffNotAvailable}}
-		{{if and .IsShowingOnlySingleCommit .PageIsPullFiles}}
-			<div class="ui info message">
-				<div>{{ctx.Locale.Tr "repo.pulls.showing_only_single_commit" (ShortSha .AfterCommitID)}} - <a href="{{$.Issue.Link}}/files?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}">{{ctx.Locale.Tr "repo.pulls.show_all_commits"}}</a></div>
-			</div>
+		{{if and .PageIsPullFiles .SingleCommit}}
+			{{template "repo/diff/single_commit_bar" .}}
 		{{else if and (not .IsShowingAllCommits) .PageIsPullFiles}}
 			<div class="ui info message">
 				<div>{{ctx.Locale.Tr "repo.pulls.showing_specified_commit_range" (ShortSha .BeforeCommitID) (ShortSha .AfterCommitID)}} - <a href="{{$.Issue.Link}}/files?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}">{{ctx.Locale.Tr "repo.pulls.show_all_commits"}}</a></div>

templates/repo/diff/comment_form.tmpl

@@ -4,7 +4,7 @@
 {{if $canAddComment}}
 	<form class="ui form {{if $replyReviewComment}}tw-hidden comment-form{{end}}" action="{{$root.Issue.Link}}/files/reviews/comments" method="post">
 		<input type="hidden" name="origin" value="{{if $root.PageIsPullFiles}}diff{{else}}timeline{{end}}">
-		<input type="hidden" name="latest_commit_id" value="{{$root.AfterCommitID}}">
+		<input type="hidden" name="latest_commit_id" value="{{$root.ReviewCommitID}}">
 		<input type="hidden" name="side" value="{{$replyReviewComment.DiffSide}}">
 		<input type="hidden" name="line" value="{{$replyReviewComment.UnsignedLine}}">
 		<input type="hidden" name="path" value="{{$replyReviewComment.TreePath}}">

templates/repo/diff/new_review.tmpl

@@ -1,8 +1,8 @@
 {{$disabledReviewTooltip := "" -}}
 {{if $.Repository.IsArchived -}}
 	{{$disabledReviewTooltip = ctx.Locale.Tr "repo.archive.pull.nocomment" -}}
-{{else if not $.IsShowingAllCommits -}}
-	{{$disabledReviewTooltip = ctx.Locale.Tr "repo.pulls.review_only_possible_for_full_diff" -}}
+{{else if not $.CanSubmitReview -}}
+	{{$disabledReviewTooltip = ctx.Locale.Tr "repo.pulls.review_not_possible_for_commit_range" -}}
 {{end -}}
 <div data-global-init="initPullReviewBox">
 <div {{if $disabledReviewTooltip}}data-tooltip-content="{{$disabledReviewTooltip}}"{{end}}>
@@ -12,13 +12,13 @@
 		{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 	</button>
 </div>
-{{if $.IsShowingAllCommits}}
+{{if $.CanSubmitReview}}
 <div class="review-box-panel tippy-target">
 	<div class="ui segment">
 		<form class="ui form form-fetch-action" action="{{.Issue.Link}}/files/reviews/submit" method="post">
-			<input type="hidden" name="commit_id" value="{{.AfterCommitID}}">
+			<input type="hidden" name="commit_id" value="{{.ReviewCommitID}}">
 			<div class="field flex-text-block">
-				<div class="tw-flex-1">{{ctx.Locale.Tr "repo.diff.review.header"}}</div>
+				<b class="tw-flex-1">{{ctx.Locale.Tr "repo.diff.review.header"}}</b>
 				<a class="muted close-review-panel">{{svg "octicon-x" 16}}</a>
 			</div>
 			<div class="field">

templates/repo/diff/single_commit_bar.tmpl

@@ -0,0 +1,33 @@
+{{$queryParams := QueryBuild "?" "style" (Iif $.IsSplitStyle "split" "unified") "whitespace" $.WhitespaceBehavior "show-outdated" (Iif $.ShowOutdatedComments "true" "false")}}
+{{$commit := $.SingleCommit}}{{$commitID := $commit.ID.String}}
+{{$prevID := $.SingleCommitPrevID}}{{$nextID := $.SingleCommitNextID}}
+{{$repoLink := $.RepoLink}}
+{{$prLink := $.Issue.Link}}
+<div class="ui info message flex-left-right" data-commit-single-index="{{$.SingleCommitIndex}}" data-commit-total-count="{{$.SingleCommitCount}}">
+	<div class="flex-relaxed-list">
+		<div class="flex-text-block tw-flex-wrap">
+			{{svg "octicon-git-commit"}}
+			<a class="tw-font-mono muted" href="{{$repoLink}}/commit/{{PathEscape $commitID}}">{{ShortSha $commitID}}</a>
+			<strong class="gt-ellipsis">{{$commit.MessageTitle | ctx.RenderUtils.RenderEmoji}}</strong>
+		</div>
+		<div class="flex-text-block tw-flex-wrap tw-text-text-light-2">
+			<div class="flex-text-block">
+				{{ctx.AvatarUtils.AvatarByEmail $commit.Author.Email $commit.Author.Name 16}}
+				<span class="gt-ellipsis">{{$commit.Author.Name}}</span>
+			</div>
+			{{DateUtils.TimeSince $commit.Author.When}}
+		</div>
+	</div>
+	<div class="flex-text-block tw-flex-wrap">
+		<span>{{ctx.Locale.Tr "repo.pulls.commit_position" $.SingleCommitIndex $.SingleCommitCount}}</span>
+		<div class="ui compact icon buttons hover-opaque">
+			<a class="ui tiny basic button js-previous-commit-link {{if not $.SingleCommitPrevID}}disabled{{end}}" data-tooltip-content="{{ctx.Locale.Tr "repo.pulls.previous_commit"}}"
+				{{if $prevID}}href="{{$prLink}}/commits/{{PathEscape $prevID}}{{$queryParams}}"{{end}}
+			>{{svg "octicon-chevron-left"}}</a>
+			<a class="ui tiny basic button js-next-commit-link {{if not $nextID}}disabled{{end}}" data-tooltip-content="{{ctx.Locale.Tr "repo.pulls.next_commit"}}"
+				{{if $nextID}}href="{{$prLink}}/commits/{{PathEscape $nextID}}{{$queryParams}}"{{end}}
+			>{{svg "octicon-chevron-right"}}</a>
+		</div>
+		<a class="ui tiny basic button hover-opaque" href="{{$prLink}}/files{{$queryParams}}">{{ctx.Locale.Tr "repo.pulls.show_all_commits"}}</a>
+	</div>
+</div>

tests/integration/pull_diff_test.go

@@ -25,27 +25,51 @@ func TestPullDiff(t *testing.T) {
 	t.Run("CompletePRDiff", func(t *testing.T) {
 		testPullDiffAssertPage(t, "/user2/commitsonpr/pulls/1/files", false, []string{"test1.txt", "test10.txt", "test2.txt", "test3.txt", "test4.txt", "test5.txt", "test6.txt", "test7.txt", "test8.txt", "test9.txt"})
 	})
-	t.Run("SingleCommitPRDiff", func(t *testing.T) {
-		testPullDiffAssertPage(t, "/user2/commitsonpr/pulls/1/commits/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test3.txt"})
-	})
 	t.Run("CommitRangePRDiff", func(t *testing.T) {
 		testPullDiffAssertPage(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", true, []string{"test2.txt", "test3.txt", "test4.txt"})
 	})
-	t.Run("SingleHeadCommitReviewFormAction", testPullDiffSingleHeadCommitReviewFormAction)
+	t.Run("SingleCommit", testPullDiffSingleCommit)
 }
 
-func testPullDiffSingleHeadCommitReviewFormAction(t *testing.T) {
+func testPullDiffSingleCommit(t *testing.T) {
 	session := loginUser(t, "user2")
-	req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/1978192d98bb1b65e11c2cf37da854fbf94bffd6")
+	req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2")
 	resp := session.MakeRequest(t, req, http.StatusOK)
 	doc := NewHTMLParser(t, resp.Body)
 
-	btn := doc.Find(".review-box-button")
-	require.Equal(t, 1, btn.Length())
-	assert.False(t, btn.Is("[disabled]"))
-	form := doc.Find(".review-box-panel form")
-	assert.Equal(t, 1, form.Length())
-	assert.Equal(t, "/user2/commitsonpr/pulls/1/files/reviews/submit", form.AttrOr("action", ""))
+	bar := doc.Find("[data-commit-single-index]")
+	require.Equal(t, 1, bar.Length())
+	barText := bar.Text()
+	assert.Contains(t, barText, "test 3")
+	assert.Contains(t, barText, "Sebastian Sauer")
+	assert.Equal(t, "3", bar.AttrOr("data-commit-single-index", ""))
+	assert.Equal(t, "10", bar.AttrOr("data-commit-total-count", ""))
+
+	assert.Contains(t, bar.Find(".js-previous-commit-link").AttrOr("href", ""), "/pulls/1/commits/96cef4a7b72b3c208340ae6f0cf55a93e9077c93")
+	assert.Contains(t, bar.Find(".js-next-commit-link").AttrOr("href", ""), "/pulls/1/commits/23576dd018294e476c06e569b6b0f170d0558705")
+
+	commitSelect := doc.Find("#diff-commit-select")
+	assert.Equal(t, "96cef4a7b72b3c208340ae6f0cf55a93e9077c93", commitSelect.AttrOr("data-before-commit", ""))
+	assert.Equal(t, "c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", commitSelect.AttrOr("data-after-commit", ""))
+
+	panel := doc.Find(".review-box-panel form")
+	require.Equal(t, 1, panel.Length())
+	assert.Equal(t, "c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", panel.Find(`input[name="commit_id"]`).AttrOr("value", ""))
+
+	req = NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/files/reviews/new_comment")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, "1978192d98bb1b65e11c2cf37da854fbf94bffd6", NewHTMLParser(t, resp.Body).Find(`input[name="latest_commit_id"]`).AttrOr("value", ""))
+
+	req = NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/4ca8bcaf27e28504df7bf996819665986b01c847")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	bar = NewHTMLParser(t, resp.Body).Find("[data-commit-single-index]")
+	assert.Equal(t, "1", bar.AttrOr("data-commit-single-index", ""))
+	assert.True(t, bar.Find(".js-previous-commit-link").HasClass("disabled"))
+	assert.False(t, bar.Find(".js-next-commit-link").HasClass("disabled"))
+
+	req = NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/cbff181af4c9c7fee3cf6c106699e07d9a3f54e6")
+	resp = session.MakeRequest(t, req, http.StatusSeeOther)
+	assert.Equal(t, "/user2/commitsonpr/commit/cbff181af4c9c7fee3cf6c106699e07d9a3f54e6", test.RedirectURL(resp))
 }
 
 func testPullDiffAssertPage(t *testing.T, prDiffURL string, reviewBtnDisabled bool, expectedFilenames []string) {

web_src/css/modules/button.css

@@ -32,7 +32,11 @@
   color: var(--color-text);
 }
 
-.ui.button.hover-opaque:hover {
+.ui.ui.ui.buttons.hover-opaque {
+  background: var(--color-body)
+}
+
+.ui.ui.ui.button.hover-opaque:hover {
   /* ".ui.button:hover" uses alpha channel to "highlight", it doesn't work when the background is not body: the inherited color is not right
    for such case, use this opaque color */
   background: var(--color-hover-opaque);
@@ -282,6 +286,7 @@
   font-size: 0;
   vertical-align: baseline;
   margin: 0 0.25em 0 0;
+  border-radius: var(--border-radius);
 }
 
 /* btn is a plain button without any opinionated styling, it only uses flex for vertical alignment like ".ui.button" in base.css */

web_src/js/components/DiffCommitSelector.vue

@@ -23,11 +23,14 @@ type CommitListResult = {
 const elRoot = useTemplateRef('elRoot') as Readonly<ShallowRef<HTMLDivElement>>;
 const elExpandBtn = useTemplateRef('elExpandBtn') as Readonly<ShallowRef<HTMLButtonElement>>;
 const elShowAllChanges = useTemplateRef('elShowAllChanges') as Readonly<ShallowRef<HTMLDivElement>>;
+const elMenu = useTemplateRef('elMenu') as Readonly<ShallowRef<HTMLDivElement>>;
 
 const elMount = document.querySelector('#diff-commit-select')!;
 const queryParams = elMount.getAttribute('data-queryparams');
 const issueLink = elMount.getAttribute('data-issuelink');
 const mergeBase = elMount.getAttribute('data-merge-base');
+const beforeCommit = elMount.getAttribute('data-before-commit');
+const afterCommit = elMount.getAttribute('data-after-commit');
 const uniqueIdMenu = generateElemId('diff-commit-selector-menu-');
 const uniqueIdShowAll = generateElemId('diff-commit-selector-show-all-');
 
@@ -37,6 +40,11 @@ const locale = shallowRef<Record<string, string>>({filter_changes_by_commit: elM
 const commits = ref<Array<Commit>>([]); // deep, the commit objects are mutated in place
 const hoverActivated = shallowRef(false);
 const lastReviewCommitSha = shallowRef<string | null>(null);
+const activeRange = shallowRef<[number, number] | null>(null);
+
+function isActive(idx: number) {
+  return activeRange.value !== null && activeRange.value[0] <= idx && idx <= activeRange.value[1];
+}
 
 const commitsSinceLastReview = computed(() => {
   if (lastReviewCommitSha.value) {
@@ -140,6 +148,8 @@ async function toggleMenu() {
   nextTick(() => {
     if (menuVisible.value) {
       focusElem(elShowAllChanges.value, elExpandBtn.value);
+      const elActive = elMenu.value.querySelector<HTMLElement>('.item.active');
+      if (elActive) elMenu.value.scrollTop = elActive.offsetTop - elMenu.value.clientHeight / 2; // scrollIntoView would also scroll the page
     } else {
       focusElem(elExpandBtn.value, elShowAllChanges.value);
     }
@@ -160,6 +170,8 @@ async function fetchCommits() {
     lastReviewCommitSha.value = null;
   }
   locale.value = {...locale.value, ...results.locale};
+  const end = commits.value.findIndex((x) => x.id === afterCommit);
+  if (end >= 0) activeRange.value = [commits.value.findIndex((x) => x.id === beforeCommit) + 1, end]; // starts at 0 when beforeCommit is outside the list
 }
 
 function showAllChanges() {
@@ -228,7 +240,7 @@ function commitClickedShift(commit: Commit) {
       <svg-icon name="octicon-git-commit"/>
     </button>
     <!-- this dropdown is not managed by Fomantic UI, so it needs some classes like "transition" explicitly -->
-    <div class="left menu transition" :id="uniqueIdMenu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
+    <div class="left menu transition" ref="elMenu" :id="uniqueIdMenu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
       <div class="loading-indicator is-loading" v-if="isLoading"/>
       <div v-if="!isLoading" class="item" :id="uniqueIdShowAll" ref="elShowAllChanges" role="menuitem" @keydown.enter="showAllChanges()" @click="showAllChanges()">
         <div class="gt-ellipsis">
@@ -257,7 +269,8 @@ function commitClickedShift(commit: Commit) {
       <template v-for="(commit, idx) in commits" :key="commit.id">
         <div
           class="item" role="menuitem"
-          :class="{selected: commit.selected, hovered: commit.hovered}"
+          :class="{selected: commit.selected, hovered: commit.hovered, active: isActive(idx)}"
+          :aria-current="isActive(idx) || undefined"
           :data-commit-idx="idx"
           @keydown.enter.exact="commitClicked(commit.id)"
           @keydown.enter.shift.exact="commitClickedShift(commit)"
@@ -279,7 +292,8 @@ function commitClickedShift(commit: Commit) {
               </span>
             </div>
           </div>
-          <div class="tw-font-mono">
+          <div class="tw-font-mono flex-text-block">
+            <svg-icon name="octicon-check" :size="14" v-if="isActive(idx)"/>
             {{ commit.short_sha }}
           </div>
         </div>
@@ -329,6 +343,10 @@ function commitClickedShift(commit: Commit) {
     background-color: var(--color-accent);
   }
 
+  .ui.dropdown.diff-commit-selector .menu > .item.active {
+    font-weight: var(--font-weight-medium);
+  }
+
   .ui.dropdown.diff-commit-selector .menu .commit-list-summary {
     max-width: min(380px, 96vw);
   }