feat: support multiple choice branches by Chzxxuanzheng · Pull Request #45 · asispts/neo-git-graph

Chzxxuanzheng · 2026-04-11T09:33:12Z

Q	A
Type	feature

Add the supporting of multiple choice branches

Modification

Extract the AbstractDropdown class from Dropdown, and add a new class CheckboxDropdown extends AbstractDropdown.
Add a constant variate SHOW_ALL instead of "" to improve the readability.
switch branchDropdown to CheckboxDropdown instance to support multiple choice
modify the currentBranch: string to currentBranches: string[] to support multiple choice
modify the query's logic to enable get multiple branches commits at one time

Issue

The choice bar will overflow if the choice branches is too many. This because the old .dropdownCurrentValue don't consider the text overflow.

The goal of the PR is add supporting about multiple choice branches, rather fix the bug of CSS. So I may open a PR, which goal is fix the CSS, later

Copilot

Pull request overview

Adds multi-branch selection support to the Git graph view by switching the branch selector to a checkbox-based dropdown and updating the loadCommits query to accept multiple branches.

Changes:

Refactor dropdown UI: extract AbstractDropdown and introduce CheckboxDropdown for multi-select.
Update webview state + messaging from currentBranch: string / branchName: string to currentBranches: string[] / branchNames: string[].
Update backend loadCommits query to accept multiple branches and pass them through to git log; adjust related tests and types.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/backend/queries/loadCommits/list.test.ts	Updates query inputs to `branchNames` and adds a local `SHOW_ALL` sentinel for readability.
src/webview/main.ts	Switches branch selector to `CheckboxDropdown`, tracks `currentBranches`, and sends `branchNames` in loadCommits requests.
src/webview/global.d.ts	Updates webview persisted state typing to `currentBranches`.
src/webview/dropdown.ts	Introduces `AbstractDropdown` + new `CheckboxDropdown` implementation.
src/extension/messageHandler.ts	Passes `branchNames` through from webview to backend query.
src/backend/types/queries.types.ts	Updates loadCommits query request type to `branchNames: string[]`.
src/backend/queries/loadCommits.ts	Updates log retrieval to support multiple branches and the “show all” sentinel behavior.

Comments suppressed due to low confidence (1)

src/backend/queries/loadCommits.ts:119

In loadCommits, the destructuring renames branchNames to branchName, but the value is a string[]. This makes the code harder to read and easier to misuse (it’s later passed to getLog as an array). Rename the local variable to branchNames (or similar) to match its type and meaning.

  const {
    branchNames: branchName,
    maxCommits,
    showRemoteBranches,
    hard,
    dateType,
    showUncommittedChanges
  } = input;

  const [rawCommits, refData] = await Promise.all([
    getLog(git, branchName, maxCommits + 1, showRemoteBranches, dateType),
    getRefs(git, showRemoteBranches)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

asispts

Can you revert the formatting-only change? It adds too much noise and makes the actual changes hard to review. I’ll review the feature after that’s done.

Also, could you add some black-box tests to verify it?

Copilot

Pull request overview

This PR adds multi-select branch filtering to the GitGraph webview by introducing a checkbox-based dropdown UI and updating the loadCommits query contract to accept multiple branch names.

Changes:

Refactor dropdown implementation to support multi-select via CheckboxDropdown (extracted from shared AbstractDropdown).
Switch webview state/query flow from currentBranch: string / branchName to currentBranches: string[] / branchNames.
Update and extend tests for the new branch-selection behavior and multi-branch commit querying.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/webview/setup.ts	Updates the test DOM scaffold for the commit table structure.
tests/webview/rendering.test.ts	Adds UI tests for multi-select branch dropdown behavior.
tests/backend/queries/loadCommits/list.test.ts	Updates query tests for `branchNames` and adds a multi-branch test.
src/webview/main.ts	Switches branch selection/state/querying to multi-select (`branchNames`).
src/webview/global.d.ts	Updates persisted webview state typing to `currentBranches`.
src/webview/dropdown.ts	Introduces `AbstractDropdown` + `CheckboxDropdown` for multi-select UI.
src/extension/messageHandler.ts	Forwards `branchNames` to backend loadCommits query.
src/backend/types/queries.types.ts	Updates loadCommits request type to `branchNames: string[]`.
src/backend/queries/loadCommits.ts	Implements multi-branch `git log` invocation logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

asispts · 2026-04-13T05:13:13Z

+  if (branches.length === 1 && branches[0] === "") {
    args.push("--branches", "--tags");
    if (showRemoteBranches) args.push("--remotes");
+  } else {
+    args.push(...branches);


@Chzxxuanzheng This is a valid concern. When someone passes ["", "main"], the input will go directly into the else branch. I think the best solution is to use a discriminated union so that the input is normalized before reaching the backend.

asispts · 2026-04-13T05:20:14Z

+      (values, change) => {
+        // special handling show all
+
+        if (values.length === 0) {
+          // no selected, should add show all / current branch to selected
+          let defaultBranch: string;
+          if (this.config.showCurrentBranchByDefault && this.gitBranchHead)
+            defaultBranch = this.gitBranchHead;
+          else defaultBranch = SHOW_ALL;
+          this.branchDropdown.setSelected(defaultBranch);
+          values = [defaultBranch];
+        } else if (change === SHOW_ALL && values.includes(SHOW_ALL)) {
+          // when selected show all, should clear other selections and keep show all selected
+          const options = [{ name: l10n.showAll, value: SHOW_ALL }];
+          for (const branch of this.gitBranches) {
+            options.push({
+              name: branch.startsWith("remotes/") ? branch.substring(8) : branch,
+              value: branch
+            });
+          }
+          this.branchDropdown.setOptions(options, [SHOW_ALL]);
+          values = [SHOW_ALL];
+        } else if (change !== SHOW_ALL && values.includes(SHOW_ALL)) {
+          // when selected another, should remove show all from selected
+          this.branchDropdown.setUnSelected(SHOW_ALL);
+          values = values.filter((v) => v !== SHOW_ALL);
+        }


I don't think main.ts should care about the component behavior

asispts · 2026-04-13T05:36:59Z

+    if (!this.options.some((i) => i.value === value))
+      throw new Error(
+        `unknown option: "${value}", available options: ${this.options.map((i) => `"${i.value}"`).join(", ")}`
+      );


Suggested change

if (!this.options.some((i) => i.value === value))

throw new Error(

`unknown option: "${value}", available options: ${this.options.map((i) => `"${i.value}"`).join(", ")}`

);

if (!this.options.some((i) => i.value === value)) {

console.error(`CheckboxDropdown.setSelected: unknown option "${value}"`);

return;

}

The exception will crash the UI.

emm...I think adding a global error popup would be better. If we just use console, the user won't notice anything unless they open the DevTools. However, I'm worried that adding too much might make the PR go out of scope.

asispts · 2026-04-13T05:37:29Z

+    if (!this.options.some((i) => i.value === value))
+      throw new Error(
+        `unknown option: "${value}", available options: ${this.options.map((i) => `"${i.value}"`).join(", ")}`
+      );


Suggested change

if (!this.options.some((i) => i.value === value))

throw new Error(

`unknown option: "${value}", available options: ${this.options.map((i) => `"${i.value}"`).join(", ")}`

);

if (!this.options.some((i) => i.value === value)) {

console.error(`CheckboxDropdown.setUnSelected: unknown option "${value}"`);

return;

}

asispts · 2026-04-13T05:42:37Z

+      expect(resultBranch1And2.commits.length).toBeGreaterThan(0);
+      expect(branch1And2Messages).toContain("commit on branch1");
+      expect(branch1And2Messages).toContain("commit on branch2");
+      expect(resultAllBranches.commits.length).toBeGreaterThanOrEqual(
+        resultBranch1And2.commits.length
+      );
+      expect(resultBranch1And2.moreCommitsAvailable).toBe(false);


The test does not confirm that commits from unselected branches never appear.

asispts · 2026-04-13T05:47:31Z

+const SHOW_ALL = "";
+


We now have two sources of truth to maintain. Just remove this one.

Is there a share file that can be imported by webview and backend? I think replace "" with SHOW_ALL is a better

feat: support multiple choice branches

b430100

Chzxxuanzheng requested a review from asispts as a code owner April 11, 2026 09:33

Copilot AI review requested due to automatic review settings April 11, 2026 09:33

Copilot started reviewing on behalf of Chzxxuanzheng April 11, 2026 09:33 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread src/webview/main.ts

Comment thread src/backend/queries/loadCommits.ts

Comment thread src/webview/dropdown.ts

Comment thread src/webview/main.ts

fix: some small bug feedback by copilot

50a791d

asispts requested changes Apr 12, 2026

View reviewed changes

Chzxxuanzheng added 2 commits April 12, 2026 20:35

revert: useless format

3a49ed5

test: add test for multiple choice branches

5695dfb

Copilot AI review requested due to automatic review settings April 12, 2026 13:34

Copilot started reviewing on behalf of Chzxxuanzheng April 12, 2026 13:34 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

fix: fix some issues feedback by copilot

552222e

Chzxxuanzheng requested a review from asispts April 13, 2026 04:23

asispts requested changes Apr 13, 2026

View reviewed changes

Conversation

Chzxxuanzheng commented Apr 11, 2026

Modification

Issue

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asispts left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asispts left a comment •

edited

Loading