Skip to content

Commit

Permalink
Bugfixes and deduplicate cleanup (#6024)
Browse files Browse the repository at this point in the history
  • Loading branch information
fregante committed Oct 2, 2022
1 parent 9c5fbc6 commit 7c352ef
Show file tree
Hide file tree
Showing 27 changed files with 94 additions and 152 deletions.
5 changes: 1 addition & 4 deletions source/features/bypass-checks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ async function bypass(detailsLink: HTMLAnchorElement): Promise<void> {
return;
}

// TODO: Use v4: https://docs.github.com/en/graphql/reference/objects#checkrun
const {details_url: detailsUrl} = await api.v3(`check-runs/${runId}`);
if (!detailsUrl) {
return;
Expand Down Expand Up @@ -46,10 +47,6 @@ void features.add(import.meta.url, {
include: [
pageDetect.isRepo,
],
exclude: [
pageDetect.isEmptyRepo,
],
deduplicate: 'has-rgh-inner',
awaitDomReady: false,
init,
});
19 changes: 8 additions & 11 deletions source/features/clean-conversation-sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as pageDetect from 'github-url-detection';

import features from '../feature-manager';
import onElementRemoval from '../helpers/on-element-removal';
import onDiscussionSidebarUpdate from '../github-events/on-discussion-sidebar-update';
import observe from '../helpers/selector-observer';

const canEditSidebar = onetime((): boolean => select.exists('.discussion-sidebar-item [data-hotkey="l"]'));

Expand Down Expand Up @@ -64,11 +64,7 @@ function cleanSection(selector: string): boolean {
return true;
}

async function init(signal: AbortSignal): Promise<void> {
if (select.exists('.rgh-clean-sidebar')) {
return;
}

async function cleanSidebar(): Promise<void> {
select('#partial-discussion-sidebar')!.classList.add('rgh-clean-sidebar');

// Assignees
Expand All @@ -90,7 +86,8 @@ async function init(signal: AbortSignal): Promise<void> {
if (pageDetect.isPR()) {
const possibleReviewers = select('[src$="/suggested-reviewers"]');
if (possibleReviewers) {
await onElementRemoval(possibleReviewers, signal);
// TODO: This blocks the whole function, it should be extracted
await onElementRemoval(possibleReviewers);
}

const content = select('[aria-label="Select reviewers"] > .css-truncate')!;
Expand Down Expand Up @@ -125,13 +122,13 @@ async function init(signal: AbortSignal): Promise<void> {
cleanSection('[aria-label="Select milestones"]');
}

function init(signal: AbortSignal): void {
observe('#partial-discussion-sidebar', cleanSidebar, {signal});
}

void features.add(import.meta.url, {
include: [
pageDetect.isConversation,
],
additionalListeners: [
onDiscussionSidebarUpdate,
],
deduplicate: 'has-rgh-inner',
init,
});
16 changes: 14 additions & 2 deletions source/features/clean-rich-text-editor.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
/* Hide unnecessary comment toolbar items, but only on desktop #5743 */
/* Kinda excludes "soft keyboards" devices https://github.com/w3c/csswg-drafts/issues/3871 */
@media (hover: hover) and (pointer: fine) {
.rgh-clean-rich-text-editor :is(md-mention, md-ref, md-header, md-bold, md-italic) {
display: none !important; /* Has to override `.d-inline-block` */
.rgh-clean-rich-text-editor :is(
md-mention,
md-ref,
md-header,
md-bold,
md-italic
):not(:focus) {
/* Like GitHub’s `show-on-focus` class. Needed because we target `md-ref` with the observer in `table-input` and `collapsible-content-button` */
position: absolute;
width: 1px;
height: 1px;
margin: 0;
overflow: hidden;
clip: rect(1px, 1px, 1px, 1px);
}
}
21 changes: 8 additions & 13 deletions source/features/collapsible-content-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import delegate, {DelegateEvent} from 'delegate-it';

import features from '../feature-manager';
import smartBlockWrap from '../helpers/smart-block-wrap';
import {onCommentEdit} from '../github-events/on-fragment-load';
import {attachElements} from '../helpers/attach-element';
import observe from '../helpers/selector-observer';

function addContentToDetails({delegateTarget}: DelegateEvent<MouseEvent, HTMLButtonElement>): void {
/* There's only one rich-text editor even when multiple fields are visible; the class targets it #5303 */
Expand Down Expand Up @@ -35,20 +34,16 @@ function addContentToDetails({delegateTarget}: DelegateEvent<MouseEvent, HTMLBut
);
}

function addButtons(): void {
attachElements('md-ref', {
className: 'rgh-collapsible-content-btn',
after: () => (
<button type="button" className="toolbar-item btn-octicon p-2 p-md-1 tooltipped tooltipped-sw" aria-label="Add collapsible content">
<FoldDownIcon/>
</button>
),
});
function addButtons(referenceButton: HTMLElement): void {
referenceButton.after(
<button type="button" className="toolbar-item btn-octicon p-2 p-md-1 tooltipped tooltipped-sw rgh-collapsible-content-btn" aria-label="Add collapsible content">
<FoldDownIcon/>
</button>,
);
}

function init(signal: AbortSignal): void {
addButtons();
onCommentEdit(addButtons, signal);
observe('md-ref', addButtons, {signal});
delegate(document, '.rgh-collapsible-content-btn', 'click', addContentToDetails, {signal});
}

Expand Down
5 changes: 5 additions & 0 deletions source/features/comments-time-machine-links.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,16 @@ function addDropdownLink(menu: HTMLElement, timestamp: string): void {

function init(signal: AbortSignal): void {
observe('.timeline-comment-actions > details:last-child', menu => {
if (menu.closest('.js-pending-review-comment')) {
return;
}

// The timestamp of main review comments isn't in their header but in the timeline event above #5423
const timestamp = menu
.closest('.js-comment:not([id^="pullrequestreview-"]), .js-timeline-item')!
.querySelector('relative-time')!
.attributes.datetime.value;

addInlineLinks(menu, timestamp);
addDropdownLink(menu, timestamp);
}, {signal});
Expand Down
3 changes: 0 additions & 3 deletions source/features/create-release-shortcut.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@ void features.add(import.meta.url, {
include: [
pageDetect.isReleasesOrTags,
],
deduplicate: 'has-rgh',
init,
}, {
include: [
pageDetect.isReleasesOrTags, // If the release couldn't be published, GitHub changes the url to /releases while still being on the "New release" page
pageDetect.isNewRelease,
pageDetect.isEditingRelease,
],
deduplicate: 'has-rgh',
init: addQuickSubmit,
});
2 changes: 1 addition & 1 deletion source/features/deep-reblame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function init(signal: AbortSignal): void {
</button>,
);
}
});
}, {signal});
}

void features.add(import.meta.url, {
Expand Down
1 change: 0 additions & 1 deletion source/features/esc-to-deselect-line.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,5 @@ void features.add(import.meta.url, {
pageDetect.hasCode,
],
awaitDomReady: false,
deduplicate: 'has-rgh-inner',
init,
});
7 changes: 3 additions & 4 deletions source/features/file-finder-buffer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ function pjaxCompleteHandler(): void {
}
}

function init(): void {
window.addEventListener('turbo:visit', pjaxStartHandler);
window.addEventListener('turbo:render', pjaxCompleteHandler);
function init(signal: AbortSignal): void {
window.addEventListener('turbo:visit', pjaxStartHandler, {signal});
window.addEventListener('turbo:render', pjaxCompleteHandler, {signal});
}

void features.add(import.meta.url, {
Expand All @@ -70,6 +70,5 @@ void features.add(import.meta.url, {
isSafari,
],
awaitDomReady: false,
deduplicate: 'has-rgh',
init,
});
19 changes: 7 additions & 12 deletions source/features/hidden-review-comments-indicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import delegate, {DelegateEvent} from 'delegate-it';

import features from '../feature-manager';
import preserveScroll from '../helpers/preserve-scroll';
import {onDiffFileLoad} from '../github-events/on-fragment-load';
import onAbort from '../helpers/abort-controller';
import observe from '../helpers/selector-observer';

// When an indicator is clicked, this will show comments on the current file
const handleIndicatorClick = ({delegateTarget}: DelegateEvent): void => {
Expand Down Expand Up @@ -39,7 +39,7 @@ const addIndicator = mem((commentThread: HTMLElement): void => {
});

// Add indicator when the `show-inline-notes` class is removed (i.e. the comments are hidden)
const observer = new MutationObserver(mutations => {
const indicatorToggleObserver = new MutationObserver(mutations => {
for (const mutation of mutations) {
const file = mutation.target as HTMLElement;
const wasVisible = mutation.oldValue!.includes('show-inline-notes');
Expand All @@ -52,24 +52,19 @@ const observer = new MutationObserver(mutations => {
}
});

function observeFiles(): void {
for (const element of select.all('.file.js-file')) {
function init(signal: AbortSignal): void {
observe('.file.js-file', element => {
// #observe won't observe the same element twice
observer.observe(element, {
indicatorToggleObserver.observe(element, {
attributes: true,
attributeOldValue: true,
attributeFilter: ['class'],
});
}
}

function init(signal: AbortSignal): void {
observeFiles();
});

onDiffFileLoad(observeFiles, signal);
delegate(document, '.rgh-comments-indicator', 'click', handleIndicatorClick, {signal});

onAbort(signal, observer);
onAbort(signal, indicatorToggleObserver);
}

void features.add(import.meta.url, {
Expand Down
14 changes: 0 additions & 14 deletions source/features/highlight-deleted-and-added-files-in-diffs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import elementReady from 'element-ready';
import * as pageDetect from 'github-url-detection';

import features from '../feature-manager';
import {onDiffFileLoad} from '../github-events/on-fragment-load';
import observe from '../helpers/selector-observer';

async function loadDeferred(jumpList: Element): Promise<void> {
Expand Down Expand Up @@ -66,19 +65,6 @@ void features.add(import.meta.url, {
pageDetect.isPRCommit404,
],
awaitDomReady: false,
deduplicate: 'has-rgh-inner',
init,
}, {
include: [
pageDetect.isCompare,
],
exclude: [
() => select.exists('.blankslate:not(.blankslate-large)'),
],
additionalListeners: [
onDiffFileLoad,
],
onlyAdditionalListeners: true,
init,
});

Expand Down
8 changes: 3 additions & 5 deletions source/features/infinite-scroll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as pageDetect from 'github-url-detection';

import features from '../feature-manager';
import observe from '../helpers/selector-observer';
import onAbort from '../helpers/abort-controller';

const loadMore = debounce(() => {
const button = select('[role="tabpanel"]:not([hidden]) button.ajax-pagination-btn')!;
Expand All @@ -28,7 +29,8 @@ const inView = new IntersectionObserver(([{isIntersecting}]) => {
rootMargin: '500px', // https://github.com/refined-github/refined-github/pull/505#issuecomment-309273098
});

function init(signal: AbortSignal): Deinit {
function init(signal: AbortSignal): void {
onAbort(signal, inView);
observe('.ajax-pagination-btn', button => {
inView.observe(button);
}, {signal});
Expand All @@ -49,10 +51,6 @@ function init(signal: AbortSignal): Deinit {
{footer}
</div>,
);

return [
inView,
];
}

void features.add(import.meta.url, {
Expand Down
16 changes: 12 additions & 4 deletions source/features/jump-to-conversation-close-event.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ function addToConversation(discussionHeader: HTMLElement): void {
// Avoid native `title` by disabling pointer events, we have our own `aria-label`. We can't drop the `title` attribute because some features depend on it.
discussionHeader.style.pointerEvents = 'none';

const lastCloseEvent = select.last('.TimelineItem-badge :is(.octicon-issue-closed, .octicon-git-merge, .octicon-git-pull-request-closed, .octicon-skip)')!.closest('.TimelineItem')!;
const lastCloseEvent = select.last(`
.TimelineItem-badge :is(
.octicon-issue-closed,
.octicon-git-merge,
.octicon-git-pull-request-closed,
.octicon-skip
)
`)!.closest('.TimelineItem')!;
wrap(discussionHeader,
<a
aria-label="Scroll to most recent close event"
Expand All @@ -25,9 +32,8 @@ function init(signal: AbortSignal): void {
observe(
css`
#partial-discussion-header :is(
[title="Status: Closed"],
[title="Status: Merged"],
[title="Status: Closed as not planned"]
[title^="Status: Closed"],
[title^="Status: Merged"]
)
`,
addToConversation,
Expand All @@ -43,6 +49,8 @@ void features.add(import.meta.url, {
pageDetect.isClosedIssue,
pageDetect.isClosedPR,
],
// Can't be used because we're specifically looking for the last event
// awaitDomReady: false,
init,
});

Expand Down
1 change: 0 additions & 1 deletion source/features/link-to-github-io.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ void features.add(import.meta.url, {
pageDetect.isUserProfileRepoTab,
pageDetect.isOrganizationProfile,
],
deduplicate: 'has-rgh',
awaitDomReady: false,
init: initRepoList,
});
2 changes: 1 addition & 1 deletion source/features/one-click-diff-options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function attachPRButtons(diffSettings: HTMLElement): void {

function initPR(signal: AbortSignal): void {
// There are two "diff settings" element, one for mobile and one for the desktop. We only replace the one for the desktop
observe('.hide-sm.hide-md [aria-label="Diff settings"]', attachPRButtons);
observe('.hide-sm.hide-md [aria-label="Diff settings"]', attachPRButtons, {signal});
delegate(document, diffSwitchButtons.selector, 'click', alternateDiffNatively, {signal});
}

Expand Down
8 changes: 0 additions & 8 deletions source/features/prevent-pr-merge-panel-opening.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,9 @@ function init(signal: AbortSignal): void {
}

void features.add(import.meta.url, {
asLongAs: [
// The user is a maintainer, so they can probably merge the PR
() => select.exists('.discussion-sidebar-item .octicon-lock'),
],
include: [
pageDetect.isPRConversation,
],
exclude: [
() => select.exists('#partial-discussion-header [title="Status: Draft"]'),
],
awaitDomReady: false,
deduplicate: 'has-rgh-inner',
init,
});
1 change: 0 additions & 1 deletion source/features/show-associated-branch-prs-on-fork.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,5 @@ void features.add(import.meta.url, {
pageDetect.isBranches,
],
awaitDomReady: false,
deduplicate: 'has-rgh',
init,
});
2 changes: 1 addition & 1 deletion source/features/stop-pjax-loading-with-esc.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ function pjaxErrorHandler(event: CustomEvent): void {
}
}

// Do not use signal, these event must persist on unload
function init(): void {
progressLoader = select('.progress-pjax-loader')!;
window.addEventListener('keydown', keydownHandler);
}

void features.add(import.meta.url, {
deduplicate: 'has-rgh',
init,
});

0 comments on commit 7c352ef

Please sign in to comment.