Improve reliability of `list-prs-for-branch` (#6641)23.5.10

9 files changed, 68 insertions, 55 deletions

diff --git a/source/features/default-branch-button.tsx b/source/features/default-branch-button.tsx
index eb9258df..866f80c8 100644
--- a/source/features/default-branch-button.tsx
+++ b/source/features/default-branch-button.tsx
@@ -10,6 +10,7 @@ import getDefaultBranch from '../github-helpers/get-default-branch.js';
 import observe from '../helpers/selector-observer.js';
 import {branchSelector} from '../github-helpers/selectors.js';
 import isDefaultBranch from '../github-helpers/is-default-branch.js';
+import {isRepoCommitListRoot} from '../github-helpers/index.js';
 
 async function add(branchSelector: HTMLElement): Promise<void> {
 	// Don't show the button if we’re already on the default branch
@@ -55,7 +56,7 @@ void features.add(import.meta.url, {
 	include: [
 		pageDetect.isRepoTree,
 		pageDetect.isSingleFile,
-		pageDetect.isRepoCommitList,
+		isRepoCommitListRoot,
 	],
 	exclude: [
 		pageDetect.isRepoHome,
diff --git a/source/features/list-prs-for-branch.tsx b/source/features/list-prs-for-branch.tsx
index 93986967..ae61b6f4 100644
--- a/source/features/list-prs-for-branch.tsx
+++ b/source/features/list-prs-for-branch.tsx
@@ -1,12 +1,13 @@
 import React from 'dom-chef';
-import * as pageDetect from 'github-url-detection';
 
 import features from '../feature-manager.js';
 import getCurrentGitRef from '../github-helpers/get-current-git-ref.js';
 import isDefaultBranch from '../github-helpers/is-default-branch.js';
-import addAfterBranchSelector from '../helpers/add-after-branch-selector.js';
 import {getPullRequestsAssociatedWithBranch, stateIcon} from './show-associated-branch-prs-on-fork.js';
-import {isPermalink} from '../github-helpers/index.js';
+import {addAfterBranchSelector, isPermalink, isRepoCommitListRoot} from '../github-helpers/index.js';
+import observe from '../helpers/selector-observer.js';
+import api from '../github-helpers/api.js';
+import {branchSelectorParent} from '../github-helpers/selectors.js';
 
 // Taken from https://github.com/fregante/github-issue-link-status/blob/98792f2837352bacbf80664f3edbcec8e579ed17/source/github-issue-link-status.js#L10
 const stateColorMap = {
@@ -16,11 +17,7 @@ const stateColorMap = {
 	DRAFT: '',
 };
 
-async function init(): Promise<void | false> {
-	if (await isPermalink() || await isDefaultBranch()) {
-		return false;
-	}
-
+async function add(branchSelectorParent: HTMLDetailsElement): Promise<void | false> {
 	const getPr = await getPullRequestsAssociatedWithBranch();
 	const currentBranch = getCurrentGitRef()!;
 	const prInfo = getPr[currentBranch];
@@ -29,7 +26,9 @@ async function init(): Promise<void | false> {
 	}
 
 	const StateIcon = stateIcon[prInfo.state];
-	const link = (
+
+	addAfterBranchSelector(
+		branchSelectorParent,
 		<a
 			data-issue-and-pr-hovercards-enabled
 			href={prInfo.url}
@@ -39,16 +38,32 @@ async function init(): Promise<void | false> {
 		>
 			<StateIcon className={stateColorMap[prInfo.state]}/>
 			<span> #{prInfo.number}</span>
-		</a>
+		</a>,
 	);
+}
 
-	await addAfterBranchSelector(link);
+async function init(signal: AbortSignal): Promise<false | void> {
+	if (await isPermalink() || await isDefaultBranch()) {
+		return false;
+	}
+
+	await api.expectToken();
+
+	observe(branchSelectorParent, add, {signal});
 }
 
 void features.add(import.meta.url, {
 	include: [
-		pageDetect.isRepoCommitList,
+		isRepoCommitListRoot,
 	],
-	deduplicate: 'has-rgh-inner',
 	init,
 });
+
+/*
+
+Test URLs
+
+https://github.com/refined-github/sandbox/commits/4679-1
+https://github.com/refined-github/sandbox/commits/branch/with/slashes
+
+*/
diff --git a/source/features/unreleased-commits.tsx b/source/features/unreleased-commits.tsx
index b19a0f76..ecab238f 100644
--- a/source/features/unreleased-commits.tsx
+++ b/source/features/unreleased-commits.tsx
@@ -6,10 +6,11 @@ import {TagIcon} from '@primer/octicons-react';
 import features from '../feature-manager.js';
 import observe from '../helpers/selector-observer.js';
 import * as api from '../github-helpers/api.js';
-import {buildRepoURL, cacheByRepo, getLatestVersionTag} from '../github-helpers/index.js';
+import {addAfterBranchSelector, buildRepoURL, cacheByRepo, getLatestVersionTag} from '../github-helpers/index.js';
 import isDefaultBranch from '../github-helpers/is-default-branch.js';
 import getDefaultBranch from '../github-helpers/get-default-branch.js';
 import pluralize from '../helpers/pluralize.js';
+import {branchSelectorParent} from '../github-helpers/selectors.js';
 
 type RepoPublishState = {
 	latestTag: string | false;
@@ -87,11 +88,7 @@ export const getRepoPublishState = cache.function('tag-ahead-by', async (): Prom
 	cacheKey: cacheByRepo,
 });
 
-async function add(branchSelector: HTMLElement): Promise<void> {
-	if (!await isDefaultBranch()) {
-		return;
-	}
-
+async function add(branchSelectorParent: HTMLDetailsElement): Promise<void> {
 	const {latestTag, aheadBy} = await getRepoPublishState();
 	const isAhead = aheadBy > 0;
 
@@ -105,10 +102,10 @@ async function add(branchSelector: HTMLElement): Promise<void> {
 			: pluralize(aheadBy, '$$ unreleased commit');
 	const label = `There are ${commitCount} since ${latestTag}`;
 
-	// TODO: use .position-relative:has(> #branch-select-menu)
-	branchSelector.closest('.position-relative')!.after(
+	addAfterBranchSelector(
+		branchSelectorParent,
 		<a
-			className="btn ml-2 px-2 tooltipped tooltipped-ne"
+			className="btn px-2 tooltipped tooltipped-ne"
 			href={buildRepoURL('compare', `${latestTag}...${await getDefaultBranch()}`)}
 			aria-label={label}
 		>
@@ -119,9 +116,13 @@ async function add(branchSelector: HTMLElement): Promise<void> {
 }
 
 async function init(signal: AbortSignal): Promise<false | void> {
+	if (!await isDefaultBranch()) {
+		return false;
+	}
+
 	await api.expectToken();
 
-	observe('#branch-select-menu', add, {signal});
+	observe(branchSelectorParent, add, {signal});
 }
 
 void features.add(import.meta.url, {
diff --git a/source/github-helpers/api.ts b/source/github-helpers/api.ts
index d0fd1043..5aaf5e4a 100644
--- a/source/github-helpers/api.ts
+++ b/source/github-helpers/api.ts
@@ -248,3 +248,6 @@ export async function getError(apiResponse: JsonObject): Promise<RefinedGitHubAP
 	error.response = apiResponse;
 	return error;
 }
+
+// Export single API object as default
+export * as default from './api.js';
diff --git a/source/github-helpers/get-current-git-ref.ts b/source/github-helpers/get-current-git-ref.ts
index 71b27222..74abb87a 100644
--- a/source/github-helpers/get-current-git-ref.ts
+++ b/source/github-helpers/get-current-git-ref.ts
@@ -1,6 +1,7 @@
 import {isRepoCommitList} from 'github-url-detection';
 import select from 'select-dom';
 
+import {extractCurrentBranchFromBranchPicker} from './index.js';
 import {branchSelector} from './selectors.js';
 
 const typesWithGitRef = new Set(['tree', 'blob', 'blame', 'edit', 'commit', 'commits', 'compare']);
@@ -10,7 +11,8 @@ const titleWithGitRef = / at (?<branch>[.\w-/]+)( · [\w-]+\/[\w-]+)?$/i;
 export default function getCurrentGitRef(): string | undefined {
 	// Note: This is not in the <head> so it's only available on AJAXed loads.
 	// It appears on every Code page except `commits` on folders/files
-	const refViaPicker = select(branchSelector)?.textContent!.trim();
+	const picker = select(branchSelector);
+	const refViaPicker = picker && extractCurrentBranchFromBranchPicker(picker);
 	if (refViaPicker) {
 		return refViaPicker;
 	}
diff --git a/source/github-helpers/get-default-branch.ts b/source/github-helpers/get-default-branch.ts
index ffb87e23..85a61040 100644
--- a/source/github-helpers/get-default-branch.ts
+++ b/source/github-helpers/get-default-branch.ts
@@ -3,7 +3,7 @@ import elementReady from 'element-ready';
 import {type RepositoryInfo} from 'github-url-detection';
 
 import * as api from './api.js';
-import {getRepo} from './index.js';
+import {extractCurrentBranchFromBranchPicker, getRepo} from './index.js';
 import {branchSelector} from './selectors.js';
 
 const isCurrentRepo = ({nameWithOwner}: RepositoryInfo): boolean => Boolean(getRepo()?.nameWithOwner === nameWithOwner);
@@ -17,8 +17,7 @@ async function fromDOM(): Promise<string | undefined> {
 	// We're on the default branch, so we can extract it from the current page. This exclusively happens on the exact pages:
 	// /user/repo
 	// /user/repo/commits (without further path)
-	const branchPicker = await elementReady(branchSelector);
-	return branchPicker!.textContent!.trim();
+	return extractCurrentBranchFromBranchPicker((await elementReady(branchSelector))!);
 }
 
 async function fromAPI(repository: RepositoryInfo): Promise<string> {
diff --git a/source/github-helpers/index.ts b/source/github-helpers/index.ts
index ed491a7b..979ffbcd 100644
--- a/source/github-helpers/index.ts
+++ b/source/github-helpers/index.ts
@@ -134,3 +134,19 @@ export async function isArchivedRepoAsync(): Promise<boolean> {
 export const userCanLikelyMergePR = (): boolean => select.exists('.discussion-sidebar-item .octicon-lock');
 
 export const cacheByRepo = (): string => getRepo()!.nameWithOwner;
+
+// Commit lists for files and folders lack a branch selector
+export const isRepoCommitListRoot = (): boolean => pageDetect.isRepoCommitList() && document.title.startsWith('Commits');
+
+// Don't make the argument optional, sometimes we really expect it to exist and want to throw an error
+export function extractCurrentBranchFromBranchPicker(branchPicker: HTMLElement): string {
+	return branchPicker.title === 'Switch branches or tags'
+		? branchPicker.textContent!.trim() // Branch name is shown in full
+		: branchPicker.title; // Branch name was clipped, so they placed it in the title attribute
+}
+
+export function addAfterBranchSelector(branchSelectorParent: HTMLDetailsElement, sibling: HTMLElement): void {
+	const row = branchSelectorParent.closest('.position-relative')!;
+	row.classList.add('d-flex', 'flex-shrink-0', 'gap-2');
+	row.append(sibling);
+}
diff --git a/source/github-helpers/selectors.ts b/source/github-helpers/selectors.ts
index d52bb04e..24663e43 100644
--- a/source/github-helpers/selectors.ts
+++ b/source/github-helpers/selectors.ts
@@ -15,6 +15,9 @@ export const branchSelector_ = [
 	'https://github.com/refined-github/sandbox/commits',
 ];
 
+export const branchSelectorParent = 'details#branch-select-menu';
+export const branchSelectorParent_ = branchSelector_;
+
 export const directoryListingFileIcon = [
 	// .color-fg-muted selects only files; some icon extensions use `img` tags
 	'.react-directory-filename-column > :is(svg, img).color-fg-muted',
diff --git a/source/helpers/add-after-branch-selector.tsx b/source/helpers/add-after-branch-selector.tsx
deleted file mode 100644
index 5ed5d35a..00000000
--- a/source/helpers/add-after-branch-selector.tsx
+++ /dev/null
@@ -1,27 +0,0 @@
-import React from 'dom-chef';
-import select from 'select-dom';
-import elementReady from 'element-ready';
-
-import {wrapAll} from './dom-utils.js';
-
-export default async function addAfterBranchSelector(button: Element): Promise<void> {
-	button.classList.add('ml-2');
-	const branchSelector = (await elementReady('#branch-select-menu', {waitForChildren: false}))!;
-	const branchSelectorWrapper = branchSelector.closest('.position-relative')!;
-	const breadcrumb = select('.breadcrumb');
-	if (!breadcrumb) {
-		branchSelectorWrapper.after(button);
-		return;
-	}
-
-	branchSelectorWrapper.append(button);
-	if (branchSelector.classList.contains('rgh-wrapper-added')) {
-		return;
-	}
-
-	breadcrumb.classList.add('flex-shrink-0');
-	breadcrumb.classList.remove('mt-3');
-	branchSelector.classList.add('rgh-wrapper-added');
-	branchSelectorWrapper.classList.add('d-flex', 'flex-shrink-0');
-	wrapAll([branchSelectorWrapper, breadcrumb], <div className="d-flex flex-wrap flex-1 mr-2" style={{rowGap: '16px'}}/>);
-}