Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster releases-tab on repo home #6909

Merged
merged 3 commits into from
Sep 14, 2023
Merged

Faster releases-tab on repo home #6909

merged 3 commits into from
Sep 14, 2023

Conversation

fregante
Copy link
Member

@fregante fregante commented Sep 12, 2023

Following #6901 I took a further look at the feature and made some changes. See self-review

It might fix #6293, so I'll close it until I see that bug again.

@fregante fregante marked this pull request as ready for review September 12, 2023 08:00
const count = pageDetect.isRepoRoot()
// Always prefer the information in the DOM
? await releasesCount.getFresh(repo)
: await releasesCount.get(repo);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm done trying to parse the same information from multiple sources. I'll only try it for the default branch. Everything else must go.

parseCountFromDom was silently failing so the tab was loading from the API at every page load on isRepoRoot, slowing the feature down.


return 0;
function detachHighlightFromCodeTab(codeTab: HTMLAnchorElement): void {
codeTab.dataset.selectedLinks = codeTab.dataset.selectedLinks!.replace('repo_releases ', '');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case it's relatively easy to have GitHub handle the tab highlight. I looked at bugs-tab as well but it's a bit more complicated:

  • we need to target and replace the contents of
<meta name="selected-link" value="repo_issues" data-turbo-transient="">

GitHub will then detect this change via MutationObserver.

Pseudo code:

in a MutationObserver
	if addedNode.value === repo_issues
		addedNode.remove()
		addedNode.value = 'repo_bugs'
		document.head.append(addedNode)

window.dispatchEvent(new Event('resize'));

const dropdownMenu = await elementReady(repoUnderlineNavDropdownUl);
triggerRepoNavOverflow();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work 🎉

@fregante fregante added the bug label Sep 12, 2023
@fregante fregante changed the title Various releases-tab improvements Faster releases-tab on repo home Sep 12, 2023
/* Only show the divider if there are other items above it */
.UnderlineNav-actions [hidden] + .dropdown-divider {
display: none;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hid the divider when releases-tab was hidden but Security is always there anyway. I don't care to support those who disable more-dropdown-links for this minor inconvenience.

@fregante fregante merged commit 98d7854 into main Sep 14, 2023
11 checks passed
@fregante fregante deleted the faster-releases-tab branch September 14, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

release-tab is added too early, out of order, sometimes
1 participant