From 3665c56818654c2c0554ad92e54a3857420e3b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Mon, 13 Jan 2025 14:27:09 +0100 Subject: [PATCH 01/16] Fixed checklist copy buttons for new UI --- src/js/lib/pages/github/issue.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index a537ebc5..ed129bd3 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -43,7 +43,7 @@ const copyReviewerChecklist = (e) => { const renderCopyChecklistButton = () => { // Look through all the comments on the page to find one that has the template for the copy/paste checklist button // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-comment-body').each((i, el) => { + $('.markdown-body > p').each((i, el) => { const commentHtml = $(el).html(); // When the button template is found, replace it with an HTML button and then put that back into the DOM so someone can click on it @@ -124,7 +124,7 @@ const refreshAssignees = () => { `); } else { - $(el).append(` + $(el).closest('li').append(` From 8a4130a856f7c6a9a958715e56a398ac86bf55b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Mon, 13 Jan 2025 18:31:30 +0100 Subject: [PATCH 02/16] Fixed k2 for new github UI --- package.json | 2 ++ src/js/lib/pages/github/_base.js | 4 ++-- src/js/lib/pages/github/issue.js | 20 ++++++++++---------- webpack.test.js | 10 ++++++++++ 4 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 webpack.test.js diff --git a/package.json b/package.json index 2b724bae..b6f2b976 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,9 @@ "preinstall": "./tools/checkRuntimeVersions.sh", "build": "npm run build:chrome", "build:chrome": "webpack --progress --config webpack.prod.js --env platform=chrome", + "build:chrome:test": "webpack --progress --config webpack.test.js --env platform=chrome", "build:firefox": "webpack --progress --config webpack.prod.js --env platform=firefox", + "build:firefox:test": "webpack --progress --config webpack.test.js --env platform=firefox", "package": "cd dist && zip -r -X ../dist.zip *", "lint": "eslint . --max-warnings=0", "lintfix": "eslint . --fix", diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index 72a7e59a..aca4443f 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -47,11 +47,11 @@ export default function () { Page.setup = function () {}; Page.getRepoOwner = function () { - return $('.author a span').text(); + return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[0].textContent.trim(); }; Page.getRepo = function () { - return $('.js-current-repository').text(); + return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[1].textContent.trim(); }; return Page; diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index ed129bd3..0dfdff61 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -13,12 +13,12 @@ import * as API from '../../api'; let clearErrorTimeoutID; function catchError(e) { - $('.gh-header-actions .k2-element').remove(); - $('.gh-header-actions').append('OOPS!'); + $('div[data-component="PH_Actions"] .k2-element').remove(); + $('.div[data-component="PH_Actions"]').append('OOPS!'); console.error(e); clearTimeout(clearErrorTimeoutID); clearErrorTimeoutID = setTimeout(() => { - $('.gh-header-actions .k2-element').remove(); + $('.div[data-component="PH_Actions"] .k2-element').remove(); }, 30000); } @@ -107,18 +107,18 @@ function replaceOwner(oldOwner, newOwner) { */ const refreshAssignees = () => { // Always start by erasing whatever was drawn before (so it always starts from a clean slate) - $('.js-issue-assignees .k2-element').remove(); + $('div[data-testid="sidebar-section"] > .k2-element').remove(); // Check if there is an owner for the issue - const ghDescription = $('.comment-body').text(); + const ghDescription = $('.markdown-body').text(); const regexResult = ghDescription.match(/Current Issue Owner:\s@(?\S+)/i); const currentOwner = regexResult && regexResult.groups && regexResult.groups.owner; // Add buttons to each assignee - $('.js-issue-assignees > p > span').each((i, el) => { - const assignee = $(el).find('.assignee span').text(); + $('div[data-testid="issue-assignees"]').each((i, el) => { + const assignee = $(el).text(); if (assignee === currentOwner) { - $(el).append(` + $(el).closest('li').append(` @@ -156,7 +156,7 @@ const refreshAssignees = () => { const refreshPicker = function () { // Add our wrappers to the DOM which all the React components will be rendered into if (!$('.k2picker-wrapper').length) { - $('.js-issue-labels').after(sidebarWrapperHTML); + $('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(3)').after(sidebarWrapperHTML) } new K2picker().draw(); @@ -197,7 +197,7 @@ export default function () { if (!$('.k2picker-wrapper').length) { refreshPicker(); } - if (!$('.js-issue-assignees .k2-element').length) { + if (!$('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(2) .k2-element').length) { refreshAssignees(); } }, 1000); diff --git a/webpack.test.js b/webpack.test.js new file mode 100644 index 00000000..7fd13a93 --- /dev/null +++ b/webpack.test.js @@ -0,0 +1,10 @@ +const {merge} = require('webpack-merge'); +const common = require('./webpack.common.js'); + +module.exports = (env) => merge(common(env), { + mode: 'development', + devtool: 'inline-source-map', + optimization: { + minimize: false + }, +}); From e803157ed9d5483da454df9a0bdb7a503a0e7ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 10:06:23 +0100 Subject: [PATCH 03/16] Fixed k2 for new github UI --- src/js/lib/pages/github/_base.js | 2 -- src/js/lib/pages/github/issue.js | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index aca4443f..f3e02c52 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -1,5 +1,3 @@ -import $ from 'jquery'; - /** * This class is to be extended by each of the distinct types of webpages that the extension works on * @returns {Object} diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index 0dfdff61..ccad9290 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -110,7 +110,7 @@ const refreshAssignees = () => { $('div[data-testid="sidebar-section"] > .k2-element').remove(); // Check if there is an owner for the issue - const ghDescription = $('.markdown-body').text(); + const ghDescription = $('.markdown-body').first().text(); const regexResult = ghDescription.match(/Current Issue Owner:\s@(?\S+)/i); const currentOwner = regexResult && regexResult.groups && regexResult.groups.owner; @@ -156,7 +156,7 @@ const refreshAssignees = () => { const refreshPicker = function () { // Add our wrappers to the DOM which all the React components will be rendered into if (!$('.k2picker-wrapper').length) { - $('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(3)').after(sidebarWrapperHTML) + $('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(3)').after(sidebarWrapperHTML); } new K2picker().draw(); From 0f09fff64603829a4e287af9160f53d85fc578bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 11:43:45 +0100 Subject: [PATCH 04/16] Fixed k2 for new github UI --- src/js/lib/pages/github/pr.js | 130 ++++++++++++++++++++++++---------- 1 file changed, 91 insertions(+), 39 deletions(-) diff --git a/src/js/lib/pages/github/pr.js b/src/js/lib/pages/github/pr.js index cc568192..5eac5536 100644 --- a/src/js/lib/pages/github/pr.js +++ b/src/js/lib/pages/github/pr.js @@ -39,7 +39,7 @@ const copyReviewerChecklist = (e) => { const renderCopyChecklistButton = () => { // Look through all the comments on the page to find one that has the template for the copy/paste checklist button // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-comment-body').each((i, el) => { + $('.markdown-body > p').each((i, el) => { const commentHtml = $(el).html(); // When the button template is found, replace it with an HTML button and then put that back into the DOM so someone can click on it @@ -55,54 +55,106 @@ const renderCopyChecklistButton = () => { const refreshHold = function () { const prTitle = $('.js-issue-title').text(); - const prAuthor = $('.pull-header-username').text(); + const prAuthor = $('.gh-header-meta .author').text(); const getCurrentUser = API.getCurrentUser(); - const branchName = $('.head-ref').text(); + const branchName = $('.head-ref span').text(); + + const isNewMerge = $('div[data-testid="mergebox-partial"]').length; + + // Classic merge experience + if (!isNewMerge) { + if (prTitle.toLowerCase().indexOf('[hold') > -1 || prTitle.toLowerCase().indexOf('[wip') > -1) { + $('.branch-action') // entire section + .removeClass('branch-action-state-clean') + .addClass('branch-action-state-dirty'); + $('.merge-message button') // merge pull request button + .removeClass('btn-primary') + .attr('disabled', 'disabled'); + // eslint-disable-next-line rulesdir/prefer-underscore-method + $('.branch-action-item').last().find('.completeness-indicator') // Last section + .removeClass('completeness-indicator-success') + .addClass('completeness-indicator-problem') + .end() + .find('.status-heading') + .text('This pull request has a hold on it and cannot be merged') + .end() + .find('.status-meta') + .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable') + .end() + .find('.octicon') + .removeClass('octicon-check') + .addClass('octicon-alert'); + } + + if (!(branchName.toLowerCase() === 'master' || branchName.toLowerCase() === 'main') && !isSelfMergingAllowed() && getCurrentUser === prAuthor) { + $('.branch-action') + .removeClass('branch-action-state-clean') + .addClass('branch-action-state-dirty'); + $('.merge-message button') + .removeClass('btn-primary') + .attr('disabled', 'disabled'); + // eslint-disable-next-line rulesdir/prefer-underscore-method + $('.branch-action-item').last().find('.completeness-indicator') + .removeClass('completeness-indicator-success') + .addClass('completeness-indicator-problem') + .end() + .find('.status-heading') + .text('You cannot merge your own PR.') + .end() + .find('.status-meta') + .html('I\'m sorry Dave, I\'m afraid you can\'t merge your own PR') + .end() + .find('.octicon') + .removeClass('octicon-check') + .addClass('octicon-alert'); + } + return; + } if (prTitle.toLowerCase().indexOf('[hold') > -1 || prTitle.toLowerCase().indexOf('[wip') > -1) { - $('.branch-action') - .removeClass('branch-action-state-clean') - .addClass('branch-action-state-dirty'); - $('.merge-message button') - .removeClass('btn-primary') + $('div[data-testid="mergebox-partial"] > div > div:last-of-type') + .removeClass('borderColor-success-emphasis'); + $('div[data-testid="mergebox-partial"] > div > div > div button').first() // merge pull request button + .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) .attr('disabled', 'disabled'); + $('div[data-testid="mergebox-partial"] > div > div button[data-component="IconButton"]').first() + .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) + .attr('disabled', 'disabled'); + $('div[data-testid="mergebox-partial"] > div > div > div > div > div') + .css({borderColor: 'var(--bgColor-neutral-muted)'}); + $('div[data-testid="mergeability-icon-wrapper"] div').css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.branch-action-item').last().find('.completeness-indicator') - .removeClass('completeness-indicator-success') - .addClass('completeness-indicator-problem') - .end() - .find('.status-heading') - .text('This pull request has a hold on it and cannot be merged') - .end() - .find('.status-meta') - .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable') - .end() - .find('.octicon') - .removeClass('octicon-check') - .addClass('octicon-alert'); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type svg') // Last section + .parent() + .removeClass('bgColor-success-emphasis') + .css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type h3') + .text('This pull request has a hold on it and cannot be merged'); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') + .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable'); } if (!(branchName.toLowerCase() === 'master' || branchName.toLowerCase() === 'main') && !isSelfMergingAllowed() && getCurrentUser === prAuthor) { - $('.branch-action') - .removeClass('branch-action-state-clean') - .addClass('branch-action-state-dirty'); - $('.merge-message button') - .removeClass('btn-primary') + $('div[data-testid="mergebox-partial"] > div > div:last-of-type') + .removeClass('borderColor-success-emphasis'); + $('div[data-testid="mergebox-partial"] > div > div > div button').first() // merge pull request button + .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) + .attr('disabled', 'disabled'); + $('div[data-testid="mergebox-partial"] > div > div button[data-component="IconButton"]').first() + .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) .attr('disabled', 'disabled'); + $('div[data-testid="mergebox-partial"] > div > div > div > div > div') + .css({borderColor: 'var(--bgColor-neutral-muted)'}); + $('div[data-testid="mergeability-icon-wrapper"] div').css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.branch-action-item').last().find('.completeness-indicator') - .removeClass('completeness-indicator-success') - .addClass('completeness-indicator-problem') - .end() - .find('.status-heading') - .text('You cannot merge your own PR.') - .end() - .find('.status-meta') - .html('I\'m sorry Dave, I\'m afraid you can\'t merge your own PR') - .end() - .find('.octicon') - .removeClass('octicon-check') - .addClass('octicon-alert'); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type svg') // Last section + .parent() + .removeClass('bgColor-success-emphasis') + .css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type h3') + .text('You cannot merge your own PR'); + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') + .html('It\'s like giving yourself a high-five in public.'); } }; From d2969a9568afc72eb2ccf0d4611efe426be4328c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 11:44:25 +0100 Subject: [PATCH 05/16] Removed leftover webpack config --- package.json | 2 -- webpack.test.js | 10 ---------- 2 files changed, 12 deletions(-) delete mode 100644 webpack.test.js diff --git a/package.json b/package.json index b6f2b976..2b724bae 100644 --- a/package.json +++ b/package.json @@ -7,9 +7,7 @@ "preinstall": "./tools/checkRuntimeVersions.sh", "build": "npm run build:chrome", "build:chrome": "webpack --progress --config webpack.prod.js --env platform=chrome", - "build:chrome:test": "webpack --progress --config webpack.test.js --env platform=chrome", "build:firefox": "webpack --progress --config webpack.prod.js --env platform=firefox", - "build:firefox:test": "webpack --progress --config webpack.test.js --env platform=firefox", "package": "cd dist && zip -r -X ../dist.zip *", "lint": "eslint . --max-warnings=0", "lintfix": "eslint . --fix", diff --git a/webpack.test.js b/webpack.test.js deleted file mode 100644 index 7fd13a93..00000000 --- a/webpack.test.js +++ /dev/null @@ -1,10 +0,0 @@ -const {merge} = require('webpack-merge'); -const common = require('./webpack.common.js'); - -module.exports = (env) => merge(common(env), { - mode: 'development', - devtool: 'inline-source-map', - optimization: { - minimize: false - }, -}); From eb8328341af474b2afc4b75e1f8f0c6c53cd1a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 11:48:15 +0100 Subject: [PATCH 06/16] Updated version numbers --- assets/manifest-firefox.json | 2 +- assets/manifest.json | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/manifest-firefox.json b/assets/manifest-firefox.json index e008db29..bc596a0b 100644 --- a/assets/manifest-firefox.json +++ b/assets/manifest-firefox.json @@ -2,7 +2,7 @@ "manifest_version": 3, "name": "K2 for GitHub", - "version": "1.3.74", + "version": "1.4.0", "description": "Manage your Kernel Scheduling from directly inside GitHub", "browser_specific_settings": { diff --git a/assets/manifest.json b/assets/manifest.json index 81329de3..605d7380 100644 --- a/assets/manifest.json +++ b/assets/manifest.json @@ -2,7 +2,7 @@ "manifest_version": 3, "name": "K2 for GitHub", - "version": "1.3.74", + "version": "1.4.0", "description": "Manage your Kernel Scheduling from directly inside GitHub", "icons": { diff --git a/package.json b/package.json index 2b724bae..469d3127 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "k2-extension", - "version": "1.3.74", + "version": "1.4.0", "description": "A Chrome Extension for Kernel Schedule", "private": true, "scripts": { From fd96fd61f073a2ce1b654f3c341ada989776c023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 11:49:14 +0100 Subject: [PATCH 07/16] Updated CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 903c8aff..6da33bfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +#1.4.0 +- Updated the extension to work with new GitHub UI +- Updated the extension to work with GitHub's new PR merge experience + #1.3.74 - Moved the previous query string params to Onyx From fbf2d89421f1366fdd76b2e1003f25f132e3084a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Tue, 14 Jan 2025 16:04:55 +0100 Subject: [PATCH 08/16] Fixed selectors --- src/js/lib/pages/github/issue.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index ccad9290..9b3c257d 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -14,11 +14,11 @@ import * as API from '../../api'; let clearErrorTimeoutID; function catchError(e) { $('div[data-component="PH_Actions"] .k2-element').remove(); - $('.div[data-component="PH_Actions"]').append('OOPS!'); + $('div[data-component="PH_Actions"]').append('OOPS!'); console.error(e); clearTimeout(clearErrorTimeoutID); clearErrorTimeoutID = setTimeout(() => { - $('.div[data-component="PH_Actions"] .k2-element').remove(); + $('div[data-component="PH_Actions"] .k2-element').remove(); }, 30000); } From 8e464d5d624a31f0db5bb23708cf0a0c19b57932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 15 Jan 2025 10:57:31 +0100 Subject: [PATCH 09/16] Fixed owner behavior --- src/js/lib/pages/github/issue.js | 100 +++++++++++++------------------ 1 file changed, 43 insertions(+), 57 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index 9b3c257d..a587892c 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -59,60 +59,46 @@ const renderCopyChecklistButton = () => { /** * Sets the owner of an issue when it doesn't have an owner yet - * @param {String} owner to set + * @param {String} newOwner to change to (removes owner if null) + * @param {String} oldOwner to change from (only adds new owner if null) */ -function setOwner(owner) { - API.getCurrentIssueDescription() - .then((response) => { - const ghDescription = response.data.body; - const newDescription = `${ghDescription} - -
Issue OwnerCurrent Issue Owner: @${owner}
`; - API.setCurrentIssueBody(newDescription); - }) - .catch(catchError); -} - -/** - * Removes the existing owner of an issue - * @param {String} owner to remove - */ -function removeOwner(owner) { - API.getCurrentIssueDescription() - .then((response) => { - const ghDescription = response.data.body; - const newDescription = ghDescription.replace(`
Issue OwnerCurrent Issue Owner: @${owner}
`, ''); - API.setCurrentIssueBody(newDescription); - }) - .catch(catchError); -} +async function setOwner(newOwner, oldOwner) { + try { + const issueDescription = (await API.getCurrentIssueDescription()).data.body; + let newDescription = ''; + + if (newOwner) { + if (oldOwner) { + newDescription = issueDescription.replace(`Current Issue Owner: @${oldOwner}`, `Current Issue Owner: @${newOwner}`); + } else { + newDescription = `${issueDescription}\n\n
Issue OwnerCurrent Issue Owner: @${newOwner}
`; + } + } else { + newDescription = issueDescription.replace(`
Issue OwnerCurrent Issue Owner: @${oldOwner}
`, ''); + } -/** - * Replaces the existing issue owner with a different owner - * @param {String} oldOwner - * @param {String} newOwner - */ -function replaceOwner(oldOwner, newOwner) { - API.getCurrentIssueDescription() - .then((response) => { - const ghDescription = response.data.body; - const newDescription = ghDescription.replace(`Current Issue Owner: @${oldOwner}`, `Current Issue Owner: @${newOwner}`); - API.setCurrentIssueBody(newDescription); - }) - .catch(catchError); + await API.setCurrentIssueBody(newDescription); + } catch (e) { + catchError(e); + } } /** * This method is all about adding the "issue owner" functionality which melvin will use to see who should be providing ksv2 updates to an issue. + * @param {String | null | undefined} issueOwner GitHub username of the issue owner. Null means no owner, undefined means get the owner from issue body */ -const refreshAssignees = () => { +const renderAssignees = (issueOwner) => { // Always start by erasing whatever was drawn before (so it always starts from a clean slate) - $('div[data-testid="sidebar-section"] > .k2-element').remove(); + $('div[data-testid="sidebar-section"] .k2-element').remove(); + + let currentOwner = issueOwner; - // Check if there is an owner for the issue - const ghDescription = $('.markdown-body').first().text(); - const regexResult = ghDescription.match(/Current Issue Owner:\s@(?\S+)/i); - const currentOwner = regexResult && regexResult.groups && regexResult.groups.owner; + // if issue owner is not provided, then try to get it from the issue body + if (currentOwner === undefined) { + const ghDescription = $('.markdown-body').first().text(); + const regexResult = ghDescription.match(/Current Issue Owner:\s@(?\S+)/i); + currentOwner = regexResult && regexResult.groups && regexResult.groups.owner; + } // Add buttons to each assignee $('div[data-testid="issue-assignees"]').each((i, el) => { @@ -133,23 +119,23 @@ const refreshAssignees = () => { }); // Remove the owner with this button is clicked - $('.k2-button-remove-owner').off('click').on('click', (e) => { + $('.k2-button-remove-owner').off('click').on('click', async (e) => { e.preventDefault(); const owner = $(e.target).data('owner'); - removeOwner(owner); - return false; + await setOwner(null, owner); + renderAssignees(null); }); // Make a new owner when this button is clicked - $('.k2-button-make-owner').off('click').on('click', (e) => { + $('.k2-button-make-owner').off('click').on('click', async (e) => { e.preventDefault(); const newOwner = $(e.target).data('owner'); if (currentOwner) { - replaceOwner(currentOwner, newOwner); + await setOwner(newOwner, currentOwner); } else { - setOwner(newOwner); + await setOwner(newOwner, null); } - return false; + renderAssignees(newOwner); }); }; @@ -172,7 +158,7 @@ const refreshPicker = function () { * @returns {Object} */ export default function () { - let allreadySetup = false; + let alreadySetup = false; ReactNativeOnyx.init({ keys: ONYXKEYS, }); @@ -183,14 +169,14 @@ export default function () { IssuePage.setup = function () { // Prevent this function from running twice (it sometimes does that because of how chrome triggers the extension) - if (allreadySetup) { + if (alreadySetup) { return; } - allreadySetup = true; + alreadySetup = true; // Draw them once when the page is loaded setTimeout(refreshPicker, 500); - setTimeout(refreshAssignees, 500); + setTimeout(renderAssignees, 500); // Every second, check to see if the pickers are still there, and if not, redraw them setInterval(() => { @@ -198,7 +184,7 @@ export default function () { refreshPicker(); } if (!$('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(2) .k2-element').length) { - refreshAssignees(); + renderAssignees(); } }, 1000); From 59ccfabcf13c7933260837ec135b3d629a91955f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 15 Jan 2025 12:17:02 +0100 Subject: [PATCH 10/16] Fixed owner behavior, cleanup + eye candy --- .eslintrc.js | 2 + src/css/content.scss | 25 +++++++ src/js/lib/pages/github/_base.js | 72 +++++++++++++++++++ src/js/lib/pages/github/createpr.js | 30 -------- src/js/lib/pages/github/issue.js | 27 ++----- src/js/lib/pages/github/pr.js | 107 +--------------------------- 6 files changed, 109 insertions(+), 154 deletions(-) delete mode 100644 src/js/lib/pages/github/createpr.js diff --git a/.eslintrc.js b/.eslintrc.js index 108c4697..b0c7a0cc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -6,6 +6,8 @@ module.exports = { 'comma-dangle': ['error', 'always-multiline'], 'rulesdir/no-api-in-views': 'off', 'rulesdir/no-multiple-api-calls': 'off', + '@lwc/lwc/no-async-await': 'off', + 'es/no-nullish-coalescing-operators' : 'off' }, settings: { 'import/resolver': { diff --git a/src/css/content.scss b/src/css/content.scss index 4d425ae8..11ad4e4a 100644 --- a/src/css/content.scss +++ b/src/css/content.scss @@ -570,3 +570,28 @@ $color-dark-yellow: #DAA520; color: rgb(230, 237, 243) } } + +.loader { + width: 14px; + aspect-ratio: 1; + border-radius: 50%; + border: 2px solid #514b82; + animation: + l20-1 0.8s infinite linear alternate, + l20-2 1.6s infinite linear; +} +@keyframes l20-1{ + 0% {clip-path: polygon(50% 50%,0 0, 50% 0%, 50% 0%, 50% 0%, 50% 0%, 50% 0% )} + 12.5% {clip-path: polygon(50% 50%,0 0, 50% 0%, 100% 0%, 100% 0%, 100% 0%, 100% 0% )} + 25% {clip-path: polygon(50% 50%,0 0, 50% 0%, 100% 0%, 100% 100%, 100% 100%, 100% 100% )} + 50% {clip-path: polygon(50% 50%,0 0, 50% 0%, 100% 0%, 100% 100%, 50% 100%, 0% 100% )} + 62.5% {clip-path: polygon(50% 50%,100% 0, 100% 0%, 100% 0%, 100% 100%, 50% 100%, 0% 100% )} + 75% {clip-path: polygon(50% 50%,100% 100%, 100% 100%, 100% 100%, 100% 100%, 50% 100%, 0% 100% )} + 100% {clip-path: polygon(50% 50%,50% 100%, 50% 100%, 50% 100%, 50% 100%, 50% 100%, 0% 100% )} +} +@keyframes l20-2{ + 0% {transform:scaleY(1) rotate(0deg)} + 49.99%{transform:scaleY(1) rotate(135deg)} + 50% {transform:scaleY(-1) rotate(0deg)} + 100% {transform:scaleY(-1) rotate(-135deg)} +} diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index f3e02c52..9fbc3886 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -1,3 +1,6 @@ +import $ from 'jquery'; +import * as API from '../../api'; + /** * This class is to be extended by each of the distinct types of webpages that the extension works on * @returns {Object} @@ -5,6 +8,54 @@ export default function () { const Page = {}; + const REVIEWER_CHECKLIST_URL = 'https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/REVIEWER_CHECKLIST.md'; + const BUGZERO_CHECKLIST_URL = 'https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/BUGZERO_CHECKLIST.md'; + + /** + * Gets the contents of the reviewer checklist from GitHub and then posts it as a comment to the current PR + * @param {Event} e + * @param {'bugzero' | 'reviewer'} checklistType Type of target checklist + */ + const copyReviewerChecklist = async (e, checklistType) => { + const checklistUrl = checklistType === 'bugzero' ? BUGZERO_CHECKLIST_URL : REVIEWER_CHECKLIST_URL; + + e.preventDefault(); + + // Get the button element + const button = e.target; + + // Save the original content of the button + const originalContent = button.innerHTML; + + // Replace the button content with a loader + button.innerHTML = '
'; + + try { + // Fetch the checklist contents + const response = await fetch(checklistUrl); + + if (!response.ok) { + console.error(`Failed to load contents of ${checklistUrl}: ${response.statusText}`); + return; + } + + const fileContents = await response.text(); + + if (!fileContents) { + console.error(`Could not load contents of ${checklistUrl} for some reason`); + return; + } + + // Call the API to add the comment + await API.addComment(fileContents); + } catch (error) { + console.error('Error fetching the checklist:', error); + } finally { + // Restore the original button content + button.innerHTML = originalContent; + } + }; + /** * A unique identifier for each page */ @@ -52,5 +103,26 @@ export default function () { return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[1].textContent.trim(); }; + /** + * Renders buttons for copying checklists in issue/PR bodies + * @param {'bugzero' | 'reviewer'} checklistType Type of target checklist + */ + Page.renderCopyChecklistButtons = function (checklistType) { + // Look through all the comments on the page to find one that has the template for the copy/paste checklist button + // eslint-disable-next-line rulesdir/prefer-underscore-method + $('.markdown-body > p').each((i, el) => { + const commentHtml = $(el).html(); + + // When the button template is found, replace it with an HTML button and then put that back into the DOM so someone can click on it + if (commentHtml && commentHtml.indexOf('you can simply click: [this button]') > -1) { + const newHtml = commentHtml.replace('[this button]', ''); + $(el).html(newHtml); + + // Now that the button is on the page, add a click handler to it (always remove all handlers first so that we know there will always be one handler attached) + $('.k2-copy-checklist').off().on('click', e => copyReviewerChecklist(e, checklistType)); + } + }); + }; + return Page; } diff --git a/src/js/lib/pages/github/createpr.js b/src/js/lib/pages/github/createpr.js deleted file mode 100644 index 7a12a3a3..00000000 --- a/src/js/lib/pages/github/createpr.js +++ /dev/null @@ -1,30 +0,0 @@ -import $ from 'jquery'; -import Base from './_base'; - -/** - * This class handles what happs on the create PR page - * the code is duplicated in pr.js so copy over any changes - * - * @returns {Object} - */ -export default function () { - const CreatePrPage = new Base(); - - /** - * Regex for the create new PR page - */ - CreatePrPage.urlPath = '^(/[\\w-]+/[\\w-]+/compare/.*)$'; - - /** - * Runs on page load, adds qa guidelines content and event listener to show/hide the guidelines - */ - CreatePrPage.setup = function () { - // eslint-disable-next-line no-undef - addQAGuidelines(); - - // eslint-disable-next-line no-undef - $('#k2-extension-qa-guidelines-toggle').on('change', toggleQAGuidelines); - }; - - return CreatePrPage; -} diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index a587892c..caffd195 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -40,32 +40,19 @@ const copyReviewerChecklist = (e) => { }); }; -const renderCopyChecklistButton = () => { - // Look through all the comments on the page to find one that has the template for the copy/paste checklist button - // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.markdown-body > p').each((i, el) => { - const commentHtml = $(el).html(); - - // When the button template is found, replace it with an HTML button and then put that back into the DOM so someone can click on it - if (commentHtml && commentHtml.indexOf('you can simply click: [this button]') > -1) { - const newHtml = commentHtml.replace('[this button]', ''); - $(el).html(newHtml); - - // Now that the button is on the page, add a click handler to it (always remove all handlers first so that we know there will always be one handler attached) - $('.k2-copy-checklist').off().on('click', copyReviewerChecklist); - } - }); -}; - /** * Sets the owner of an issue when it doesn't have an owner yet * @param {String} newOwner to change to (removes owner if null) * @param {String} oldOwner to change from (only adds new owner if null) */ async function setOwner(newOwner, oldOwner) { + const actionedOwner = newOwner ?? oldOwner; + const ownerSelector = $(`button[data-owner="${actionedOwner}"]`); + $(ownerSelector).html('
'); + try { const issueDescription = (await API.getCurrentIssueDescription()).data.body; - let newDescription = ''; + let newDescription; if (newOwner) { if (oldOwner) { @@ -85,7 +72,7 @@ async function setOwner(newOwner, oldOwner) { /** * This method is all about adding the "issue owner" functionality which melvin will use to see who should be providing ksv2 updates to an issue. - * @param {String | null | undefined} issueOwner GitHub username of the issue owner. Null means no owner, undefined means get the owner from issue body + * @param {String | null} [issueOwner] GitHub username of the issue owner. Null means no owner, undefined means get the owner from issue body */ const renderAssignees = (issueOwner) => { // Always start by erasing whatever was drawn before (so it always starts from a clean slate) @@ -189,7 +176,7 @@ export default function () { }, 1000); // Waiting 2 seconds to call this gives the page enough time to load so that there is a better chance that all the comments will be rendered - setInterval(renderCopyChecklistButton, 2000); + setInterval(() => IssuePage.renderCopyChecklistButtons(copyReviewerChecklist), 2000); }; return IssuePage; diff --git a/src/js/lib/pages/github/pr.js b/src/js/lib/pages/github/pr.js index 5eac5536..2b325131 100644 --- a/src/js/lib/pages/github/pr.js +++ b/src/js/lib/pages/github/pr.js @@ -1,68 +1,13 @@ import $ from 'jquery'; import Base from './_base'; -import * as API from '../../api'; - -/** - * Check whether or not the current repo allows someone to merge their own PR that they created. This is limited - * to specific repos in infra. - * - * @return {boolean} - */ -function isSelfMergingAllowed() { - const reposAllowSelfMerge = [ - 'salt', - 'ops-configs', - 'terraform', - ]; - const repoName = window.location.pathname.split('/')[2].toLowerCase(); - return reposAllowSelfMerge.indexOf(repoName) > -1; -} - -/** - * Gets the contents of the reviewer checklist from GitHub and then posts it as a comment to the current PR - * @param {Event} e - */ -const copyReviewerChecklist = (e) => { - e.preventDefault(); - const pathToChecklist = 'https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/REVIEWER_CHECKLIST.md'; - $.get(pathToChecklist) - .done((fileContents) => { - if (!fileContents) { - console.error(`could not load contents of ${pathToChecklist} for some reason`); - return; - } - - API.addComment(fileContents); - }); -}; - -const renderCopyChecklistButton = () => { - // Look through all the comments on the page to find one that has the template for the copy/paste checklist button - // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.markdown-body > p').each((i, el) => { - const commentHtml = $(el).html(); - - // When the button template is found, replace it with an HTML button and then put that back into the DOM so someone can click on it - if (commentHtml && commentHtml.indexOf('you can simply click: [this button]') > -1) { - const newHtml = commentHtml.replace('[this button]', ''); - $(el).html(newHtml); - - // Now that the button is on the page, add a click handler to it (always remove all handlers first so that we know there will always be one handler attached) - $('.k2-copy-checklist').off().on('click', copyReviewerChecklist); - } - }); -}; const refreshHold = function () { const prTitle = $('.js-issue-title').text(); - const prAuthor = $('.gh-header-meta .author').text(); - const getCurrentUser = API.getCurrentUser(); - const branchName = $('.head-ref span').text(); - const isNewMerge = $('div[data-testid="mergebox-partial"]').length; + const isNewMergeUI = $('div[data-testid="mergebox-partial"]').length; // Classic merge experience - if (!isNewMerge) { + if (!isNewMergeUI) { if (prTitle.toLowerCase().indexOf('[hold') > -1 || prTitle.toLowerCase().indexOf('[wip') > -1) { $('.branch-action') // entire section .removeClass('branch-action-state-clean') @@ -85,29 +30,6 @@ const refreshHold = function () { .removeClass('octicon-check') .addClass('octicon-alert'); } - - if (!(branchName.toLowerCase() === 'master' || branchName.toLowerCase() === 'main') && !isSelfMergingAllowed() && getCurrentUser === prAuthor) { - $('.branch-action') - .removeClass('branch-action-state-clean') - .addClass('branch-action-state-dirty'); - $('.merge-message button') - .removeClass('btn-primary') - .attr('disabled', 'disabled'); - // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.branch-action-item').last().find('.completeness-indicator') - .removeClass('completeness-indicator-success') - .addClass('completeness-indicator-problem') - .end() - .find('.status-heading') - .text('You cannot merge your own PR.') - .end() - .find('.status-meta') - .html('I\'m sorry Dave, I\'m afraid you can\'t merge your own PR') - .end() - .find('.octicon') - .removeClass('octicon-check') - .addClass('octicon-alert'); - } return; } @@ -133,29 +55,6 @@ const refreshHold = function () { $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable'); } - - if (!(branchName.toLowerCase() === 'master' || branchName.toLowerCase() === 'main') && !isSelfMergingAllowed() && getCurrentUser === prAuthor) { - $('div[data-testid="mergebox-partial"] > div > div:last-of-type') - .removeClass('borderColor-success-emphasis'); - $('div[data-testid="mergebox-partial"] > div > div > div button').first() // merge pull request button - .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) - .attr('disabled', 'disabled'); - $('div[data-testid="mergebox-partial"] > div > div button[data-component="IconButton"]').first() - .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) - .attr('disabled', 'disabled'); - $('div[data-testid="mergebox-partial"] > div > div > div > div > div') - .css({borderColor: 'var(--bgColor-neutral-muted)'}); - $('div[data-testid="mergeability-icon-wrapper"] div').css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); - // eslint-disable-next-line rulesdir/prefer-underscore-method - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type svg') // Last section - .parent() - .removeClass('bgColor-success-emphasis') - .css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type h3') - .text('You cannot merge your own PR'); - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') - .html('It\'s like giving yourself a high-five in public.'); - } }; /** @@ -178,7 +77,7 @@ export default function () { setInterval(refreshHold, 1000); // Waiting 2 seconds to call this gives the page enough time to load so that there is a better chance that all the comments will be rendered - setInterval(renderCopyChecklistButton, 2000); + setInterval(() => PrPage.renderCopyChecklistButtons('reviewer'), 2000); }; return PrPage; From 6eee62860b7caeceeeb7bf4fa07e2f1a9f4439bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 15 Jan 2025 16:02:49 +0100 Subject: [PATCH 11/16] Fixed checklist copy for issues --- src/js/lib/pages/github/issue.js | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index caffd195..9abd5f47 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -22,23 +22,6 @@ function catchError(e) { }, 30000); } -/** - * Gets the contents of the reviewer checklist from GitHub and then posts it as a comment to the current PR - * @param {Event} e - */ -const copyReviewerChecklist = (e) => { - e.preventDefault(); - const pathToChecklist = 'https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/BUGZERO_CHECKLIST.md'; - $.get(pathToChecklist) - .done((fileContents) => { - if (!fileContents) { - console.error(`could not load contents of ${pathToChecklist} for some reason`); - return; - } - - API.addComment(fileContents); - }); -}; /** * Sets the owner of an issue when it doesn't have an owner yet @@ -175,8 +158,10 @@ export default function () { } }, 1000); + renderLinksInTitle(); + // Waiting 2 seconds to call this gives the page enough time to load so that there is a better chance that all the comments will be rendered - setInterval(() => IssuePage.renderCopyChecklistButtons(copyReviewerChecklist), 2000); + setInterval(() => IssuePage.renderCopyChecklistButtons('bugzero'), 2000); }; return IssuePage; From 6deae9815644ca3199a58d88dddcb1386f1a7528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 15 Jan 2025 16:03:17 +0100 Subject: [PATCH 12/16] Fixed checklist copy for issues --- src/js/lib/pages/github/issue.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index 9abd5f47..211e2e56 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -22,7 +22,6 @@ function catchError(e) { }, 30000); } - /** * Sets the owner of an issue when it doesn't have an owner yet * @param {String} newOwner to change to (removes owner if null) @@ -158,8 +157,6 @@ export default function () { } }, 1000); - renderLinksInTitle(); - // Waiting 2 seconds to call this gives the page enough time to load so that there is a better chance that all the comments will be rendered setInterval(() => IssuePage.renderCopyChecklistButtons('bugzero'), 2000); }; From ab4410ba8aaef8408c3cd7bd27aaeac9272bc159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 15 Jan 2025 17:04:51 +0100 Subject: [PATCH 13/16] Update src/js/lib/pages/github/_base.js Co-authored-by: Tim Golen --- src/js/lib/pages/github/_base.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index 9fbc3886..84ef1945 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -28,7 +28,7 @@ export default function () { const originalContent = button.innerHTML; // Replace the button content with a loader - button.innerHTML = '
'; + button.innerHTML = '
'; try { // Fetch the checklist contents From 1cdb513d8423de814b79bee156894fdb996fbdc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 16 Jan 2025 11:31:00 +0100 Subject: [PATCH 14/16] Fixed panel buttons, reverted back to previous assignee behavior --- src/js/lib/pages/github/issue.js | 69 +++++++++++-------- src/js/lib/pages/github/pr.js | 1 + src/js/module/K2picker/K2PickerPicker.js | 2 +- .../module/K2pickerarea/K2PickerareaPicker.js | 2 +- .../module/K2pickertype/K2PickertypePicker.js | 2 +- src/js/module/ToggleReview/Toggle.js | 2 +- 6 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index 211e2e56..99a0e02c 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -24,32 +24,47 @@ function catchError(e) { /** * Sets the owner of an issue when it doesn't have an owner yet - * @param {String} newOwner to change to (removes owner if null) - * @param {String} oldOwner to change from (only adds new owner if null) + * @param {String} owner to set */ -async function setOwner(newOwner, oldOwner) { - const actionedOwner = newOwner ?? oldOwner; - const ownerSelector = $(`button[data-owner="${actionedOwner}"]`); - $(ownerSelector).html('
'); - - try { - const issueDescription = (await API.getCurrentIssueDescription()).data.body; - let newDescription; - - if (newOwner) { - if (oldOwner) { - newDescription = issueDescription.replace(`Current Issue Owner: @${oldOwner}`, `Current Issue Owner: @${newOwner}`); - } else { - newDescription = `${issueDescription}\n\n
Issue OwnerCurrent Issue Owner: @${newOwner}
`; - } - } else { - newDescription = issueDescription.replace(`
Issue OwnerCurrent Issue Owner: @${oldOwner}
`, ''); - } +function setOwner(owner) { + API.getCurrentIssueDescription() + .then((response) => { + const ghDescription = response.data.body; + const newDescription = `${ghDescription} + +
Issue OwnerCurrent Issue Owner: @${owner}
`; + API.setCurrentIssueBody(newDescription); + }) + .catch(catchError); +} - await API.setCurrentIssueBody(newDescription); - } catch (e) { - catchError(e); - } +/** + * Removes the existing owner of an issue + * @param {String} owner to remove + */ +function removeOwner(owner) { + API.getCurrentIssueDescription() + .then((response) => { + const ghDescription = response.data.body; + const newDescription = ghDescription.replace(`
Issue OwnerCurrent Issue Owner: @${owner}
`, ''); + API.setCurrentIssueBody(newDescription); + }) + .catch(catchError); +} + +/** + * Replaces the existing issue owner with a different owner + * @param {String} oldOwner + * @param {String} newOwner + */ +function replaceOwner(oldOwner, newOwner) { + API.getCurrentIssueDescription() + .then((response) => { + const ghDescription = response.data.body; + const newDescription = ghDescription.replace(`Current Issue Owner: @${oldOwner}`, `Current Issue Owner: @${newOwner}`); + API.setCurrentIssueBody(newDescription); + }) + .catch(catchError); } /** @@ -91,7 +106,7 @@ const renderAssignees = (issueOwner) => { $('.k2-button-remove-owner').off('click').on('click', async (e) => { e.preventDefault(); const owner = $(e.target).data('owner'); - await setOwner(null, owner); + removeOwner(owner); renderAssignees(null); }); @@ -100,9 +115,9 @@ const renderAssignees = (issueOwner) => { e.preventDefault(); const newOwner = $(e.target).data('owner'); if (currentOwner) { - await setOwner(newOwner, currentOwner); + replaceOwner(currentOwner, newOwner); } else { - await setOwner(newOwner, null); + setOwner(newOwner); } renderAssignees(newOwner); }); diff --git a/src/js/lib/pages/github/pr.js b/src/js/lib/pages/github/pr.js index 2b325131..9e63cbca 100644 --- a/src/js/lib/pages/github/pr.js +++ b/src/js/lib/pages/github/pr.js @@ -82,3 +82,4 @@ export default function () { return PrPage; } + diff --git a/src/js/module/K2picker/K2PickerPicker.js b/src/js/module/K2picker/K2PickerPicker.js index 6b26cd7c..d37f0098 100644 --- a/src/js/module/K2picker/K2PickerPicker.js +++ b/src/js/module/K2picker/K2PickerPicker.js @@ -20,7 +20,7 @@ class K2PickerPicker extends React.Component { componentDidMount() { // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-issue-labels .IssueLabel').each((i, el) => { + $('div[data-testid="issue-labels"] a > span').each((i, el) => { const label = $(el).text().trim(); if (['Hourly', 'Daily', 'Weekly', 'Monthly'].indexOf(label) > -1) { this.setActiveLabel(label); diff --git a/src/js/module/K2pickerarea/K2PickerareaPicker.js b/src/js/module/K2pickerarea/K2PickerareaPicker.js index 6266b0a9..cf9ad186 100644 --- a/src/js/module/K2pickerarea/K2PickerareaPicker.js +++ b/src/js/module/K2pickerarea/K2PickerareaPicker.js @@ -36,7 +36,7 @@ class K2PickerareaPicker extends React.Component { }, }; // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-issue-labels .IssueLabel').each((i, el) => { + $('div[data-testid="issue-labels"] a > span').each((i, el) => { const label = $(el).text().trim(); if (this.state[label]) { this.state[label].className = this.state[label].className.replace('inactive', 'active'); diff --git a/src/js/module/K2pickertype/K2PickertypePicker.js b/src/js/module/K2pickertype/K2PickertypePicker.js index 365fd6ba..d65fbea8 100644 --- a/src/js/module/K2pickertype/K2PickertypePicker.js +++ b/src/js/module/K2pickertype/K2PickertypePicker.js @@ -25,7 +25,7 @@ class K2PickertypePicker extends React.Component { */ componentDidMount() { // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-issue-labels .IssueLabel').each((i, el) => { + $('div[data-testid="issue-labels"] a > span').each((i, el) => { const label = $(el).text().trim(); if (['Improvement', 'Task', 'NewFeature'].indexOf(label) > -1) { this.setActiveLabel(label); diff --git a/src/js/module/ToggleReview/Toggle.js b/src/js/module/ToggleReview/Toggle.js index d6ee3558..266562a9 100644 --- a/src/js/module/ToggleReview/Toggle.js +++ b/src/js/module/ToggleReview/Toggle.js @@ -17,7 +17,7 @@ class Toggle extends React.Component { componentDidMount() { // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.js-issue-labels .IssueLabel').each((i, el) => { + $('div[data-testid="issue-labels"] a > span').each((i, el) => { const label = $(el).text().trim(); if (['Reviewing'].indexOf(label) > -1) { this.setActiveLabel(label); From dbf1cbba411f61b468630a9c1207f7e085ddf31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Fri, 17 Jan 2025 10:54:05 +0100 Subject: [PATCH 15/16] Added selector comments --- src/js/lib/pages/github/_base.js | 6 ++++-- src/js/lib/pages/github/issue.js | 12 ++++++++---- src/js/lib/pages/github/pr.js | 27 +++++++++++++-------------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index 84ef1945..1496951e 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -96,11 +96,13 @@ export default function () { Page.setup = function () {}; Page.getRepoOwner = function () { - return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[0].textContent.trim(); + return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[0] // Org name next to GitHub logo + .textContent.trim(); }; Page.getRepo = function () { - return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[1].textContent.trim(); + return document.querySelectorAll('.AppHeader-context-item-label.Truncate-text')[1] // Repo name next to GitHub logo + .textContent.trim(); }; /** diff --git a/src/js/lib/pages/github/issue.js b/src/js/lib/pages/github/issue.js index 99a0e02c..b636041a 100644 --- a/src/js/lib/pages/github/issue.js +++ b/src/js/lib/pages/github/issue.js @@ -13,8 +13,9 @@ import * as API from '../../api'; let clearErrorTimeoutID; function catchError(e) { - $('div[data-component="PH_Actions"] .k2-element').remove(); - $('div[data-component="PH_Actions"]').append('OOPS!'); + $('div[data-component="PH_Actions"] .k2-element').remove(); // K2 elements in action buttons + $('div[data-component="PH_Actions"]') // Action buttons next to issue title + .append('OOPS!'); console.error(e); clearTimeout(clearErrorTimeoutID); clearErrorTimeoutID = setTimeout(() => { @@ -126,7 +127,8 @@ const renderAssignees = (issueOwner) => { const refreshPicker = function () { // Add our wrappers to the DOM which all the React components will be rendered into if (!$('.k2picker-wrapper').length) { - $('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(3)').after(sidebarWrapperHTML); + $('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(3)') // Labels section in right side panel + .after(sidebarWrapperHTML); } new K2picker().draw(); @@ -167,7 +169,9 @@ export default function () { if (!$('.k2picker-wrapper').length) { refreshPicker(); } - if (!$('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(2) .k2-element').length) { + + if (!$('div[data-testid="issue-viewer-metadata-pane"] > :nth-child(2) .k2-element') // Assignee section in right side panel + .length) { renderAssignees(); } }, 1000); diff --git a/src/js/lib/pages/github/pr.js b/src/js/lib/pages/github/pr.js index 9e63cbca..e115ae8e 100644 --- a/src/js/lib/pages/github/pr.js +++ b/src/js/lib/pages/github/pr.js @@ -9,21 +9,21 @@ const refreshHold = function () { // Classic merge experience if (!isNewMergeUI) { if (prTitle.toLowerCase().indexOf('[hold') > -1 || prTitle.toLowerCase().indexOf('[wip') > -1) { - $('.branch-action') // entire section + $('.branch-action') // Entire PR merge section .removeClass('branch-action-state-clean') .addClass('branch-action-state-dirty'); - $('.merge-message button') // merge pull request button + $('.merge-message button') // Merge pull request button .removeClass('btn-primary') .attr('disabled', 'disabled'); // eslint-disable-next-line rulesdir/prefer-underscore-method - $('.branch-action-item').last().find('.completeness-indicator') // Last section + $('.branch-action-item').last().find('.completeness-indicator') // "Merging status" section above the merge button .removeClass('completeness-indicator-success') .addClass('completeness-indicator-problem') .end() - .find('.status-heading') + .find('.status-heading') // Header for the "merging status" section .text('This pull request has a hold on it and cannot be merged') .end() - .find('.status-meta') + .find('.status-meta') // Body text for the "merging status" section .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable') .end() .find('.octicon') @@ -34,25 +34,24 @@ const refreshHold = function () { } if (prTitle.toLowerCase().indexOf('[hold') > -1 || prTitle.toLowerCase().indexOf('[wip') > -1) { - $('div[data-testid="mergebox-partial"] > div > div:last-of-type') + $('div[data-testid="mergebox-partial"] > div > div:last-of-type') // Entire PR merge section .removeClass('borderColor-success-emphasis'); - $('div[data-testid="mergebox-partial"] > div > div > div button').first() // merge pull request button + $('div[data-testid="mergebox-partial"] > div > div > div button').first() // Merge pull request button .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) .attr('disabled', 'disabled'); - $('div[data-testid="mergebox-partial"] > div > div button[data-component="IconButton"]').first() + $('div[data-testid="mergebox-partial"] > div > div button[data-component="IconButton"]').first() // Dropdown button next to merge button .css({backgroundColor: 'var(--bgColor-neutral-muted)', borderColor: 'var(--bgColor-neutral-muted)'}) .attr('disabled', 'disabled'); - $('div[data-testid="mergebox-partial"] > div > div > div > div > div') + $('div[data-testid="mergebox-partial"] > div > div > div > div > div') // Container for merge pull request button .css({borderColor: 'var(--bgColor-neutral-muted)'}); - $('div[data-testid="mergeability-icon-wrapper"] div').css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); - // eslint-disable-next-line rulesdir/prefer-underscore-method - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type svg') // Last section + $('div[data-testid="mergeability-icon-wrapper"] div').css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); // Icon on the left side of the merge panel + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type svg') // "Merging status" section above the merge button .parent() .removeClass('bgColor-success-emphasis') .css({backgroundColor: 'var(--bgColor-neutral-emphasis)'}); - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type h3') + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type h3') // Header for the "merging status" section .text('This pull request has a hold on it and cannot be merged'); - $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') + $('div[data-testid="mergebox-partial"] > div > div > section:last-of-type p') // Body text for the "merging status" section .html('Remove the HOLD or WIP label from the title of the PR to make it mergeable'); } }; From 91f5531b25b42227e3b81beb4e1a85c904f18477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Fri, 17 Jan 2025 11:02:44 +0100 Subject: [PATCH 16/16] Fixed the copy checklist error handling --- src/js/lib/pages/github/_base.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/js/lib/pages/github/_base.js b/src/js/lib/pages/github/_base.js index 1496951e..ae2485b6 100644 --- a/src/js/lib/pages/github/_base.js +++ b/src/js/lib/pages/github/_base.js @@ -33,19 +33,8 @@ export default function () { try { // Fetch the checklist contents const response = await fetch(checklistUrl); - - if (!response.ok) { - console.error(`Failed to load contents of ${checklistUrl}: ${response.statusText}`); - return; - } - const fileContents = await response.text(); - if (!fileContents) { - console.error(`Could not load contents of ${checklistUrl} for some reason`); - return; - } - // Call the API to add the comment await API.addComment(fileContents); } catch (error) {