Skip to content

Fancy links for GH issues representing external reviews #3749

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

Merged
merged 4 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions client-src/elements/chromedash-link.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import {css, html, LitElement} from 'lit';
import {ifDefined} from 'lit/directives/if-defined.js';
import {SHARED_STYLES} from '../css/shared-css';
import {ExternalReviewer} from './external-reviewers';

// LINK_TYPES should be consistent with the server link_helpers.py
const LINK_TYPE_CHROMIUM_BUG = 'chromium_bug';
Expand Down Expand Up @@ -97,7 +98,7 @@ function enhanceGithubIssueLink(featureLink, text) {
}
const information = featureLink.information;
const assignee = information.assignee_login;
const createdAt = information.created_at;
const createdAt = new Date(information.created_at);
const closedAt = information.closed_at;
const updatedAt = information.updated_at;
const state = information.state;
Expand All @@ -109,6 +110,43 @@ function enhanceGithubIssueLink(featureLink, text) {
const typePath = featureLink.url.split('/').slice(-2)[0];
const type = typePath === 'issues' ? 'Issue' : typePath === 'pull' ? 'Pull Request' : typePath;

// If this issue is an external review of the feature, find the summary description.
const externalReviewer = ExternalReviewer.get(repo);
let stateDescription = undefined;
let stateVariant = undefined;
if (externalReviewer) {
for (const label of information.labels) {
const labelInfo = externalReviewer.label(label);
if (labelInfo) {
({description: stateDescription, variant: stateVariant} = labelInfo);
break;
}
}
}

if (stateVariant === undefined) {
if (state === 'open') {
const age = Date.now() - createdAt.getTime();
stateDescription = html`Opened <sl-relative-time date=${createdAt.toISOString()}>on ${_dateTimeFormat.format(createdAt)}</sl-relative-time>`;
const week = 7 * 24 * 60 * 60 * 1000;
stateVariant = 'success';
if (externalReviewer) {
// If this is an issue asking for external review, having it filed too recently is a warning
// sign, which we'll indicate using the tag's color.
if (age < 4 * week) {
stateVariant = 'warning';
} else {
// Still only neutral if the reviewer hasn't given a position yet.
stateVariant = 'neutral';
}
}
} else {
console.assert(state === 'closed');
stateDescription = 'Closed';
stateVariant = 'neutral';
}
}

if (!text) {
text = title;
}
Expand Down Expand Up @@ -169,8 +207,8 @@ function enhanceGithubIssueLink(featureLink, text) {
<sl-tooltip style="--sl-tooltip-arrow-size: 0;--max-width: 50vw;">
<div slot="content">${renderTooltipContent()}</div>
<sl-tag>
<img src="https://docs.github.com/assets/cb-600/images/site/favicon.png" alt="icon" class="icon" />
<sl-badge size="small" variant="${state === 'open' ? 'success' : 'neutral'}">${state}</sl-badge>
<img src=${externalReviewer?.icon ?? 'https://docs.github.com/assets/cb-600/images/site/favicon.png'} alt="icon" class="icon" />
<sl-badge size="small" variant=${stateVariant}>${stateDescription}</sl-badge>
${_formatLongText(`#${number} ` + text)}
</sl-tag>
</sl-tooltip>
Expand Down Expand Up @@ -337,8 +375,8 @@ export class ChromedashLink extends LitElement {
}

sl-badge::part(base) {
height: 14px;
padding: 4px;
display: inline;
padding: 0 4px;
border-width: 0;
text-transform: capitalize;
font-weight: 400;
Expand All @@ -362,6 +400,10 @@ export class ChromedashLink extends LitElement {
background-color: rgb(209,211,213);
}

sl-relative-time {
margin: 0;
}

.icon {
display: block;
width: 12px;
Expand Down
254 changes: 250 additions & 4 deletions client-src/elements/chromedash-link_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {html} from 'lit';
import './chromedash-link';
import {_dateTimeFormat} from './chromedash-link';

const DAY = 24 * 60 * 60 * 1000;

it('shows a plain link for non-cached links', async () => {
const el = await fixture(html`<chromedash-link
href="https://github.com/GoogleChrome/chromium-dashboard/issues/3007"
Expand All @@ -27,8 +29,11 @@ it('wraps the text in sl-tag when requested', async () => {
});

describe('Github issue links', () => {
// Get the 'information' object by running
// `curl -L https://api.github.com/repos/OWNER/REPO/issues/ISSUE_NUMBER`

it('shows tooltip, open state, and title', async () => {
const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000);
const yesterday = new Date(Date.now() - DAY);
const featureLinks = [{
url: 'https://github.com/GoogleChrome/chromium-dashboard/issues/3007',
type: 'github_issue',
Expand Down Expand Up @@ -94,7 +99,10 @@ describe('Github issue links', () => {
class="icon"
src="https://docs.github.com/assets/cb-600/images/site/favicon.png">
<sl-badge size="small" variant="success">
open
Opened
<sl-relative-time date="${yesterday.toISOString()}">
on ${_dateTimeFormat.format(yesterday)}
</sl-relative-time>
</sl-badge>
#3007 Issue Title
</sl-tag>
Expand All @@ -106,7 +114,7 @@ describe('Github issue links', () => {
});

it('shows closed state and title', async () => {
const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000);
const yesterday = new Date(Date.now() - DAY);
const featureLinks = [{
url: 'https://github.com/GoogleChrome/chromium-dashboard/issues/3007',
type: 'github_issue',
Expand Down Expand Up @@ -137,7 +145,7 @@ describe('Github issue links', () => {
class="icon"
src="https://docs.github.com/assets/cb-600/images/site/favicon.png">
<sl-badge size="small" variant="neutral">
closed
Closed
</sl-badge>
#3007 Issue Title
</sl-tag>
Expand All @@ -148,4 +156,242 @@ describe('Github issue links', () => {
ignoreAttributes: ['style', 'title'],
});
});

describe('external reviewers', () => {
it('shows TAG position', async () => {
const featureLinks = [{
url: 'https://github.com/w3ctag/design-reviews/issues/400',
type: 'github_issue',
information: {
'url': 'https://api.github.com/repos/w3ctag/design-reviews/issues/400',
'number': 400,
'title': 'Font Table Access API',
'user_login': 'user',
'state': 'closed',
'assignee_login': 'assignee',
'created_at': '2019-08-07T00:53:51Z',
'updated_at': '2022-08-15T16:40:59Z',
'closed_at': '2022-08-15T16:40:29Z',
'labels': ['Needs a venue', 'Provenance: Fugu', 'Resolution: unsatisfied',
'Review type: CG early review', 'Topic: fonts'],
},
}];
const el = await fixture(html`<chromedash-link
href="https://github.com/w3ctag/design-reviews/issues/400"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/w3ctag/design-reviews/issues/400"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/3874462?s=48&amp;v=4">
<sl-badge size="small" variant="danger">
Unsatisfied
</sl-badge>
#400 Font Table Access API
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});

it('shows WebKit position', async () => {
const featureLinks = [{
url: 'https://github.com/WebKit/standards-positions/issues/268',
type: 'github_issue',
information: {
'url': 'https://api.github.com/repos/WebKit/standards-positions/issues/268',
'number': 268,
'title': 'Cross-Origin Embedder Policies - "credentialless"',
'user_login': 'user',
'state': 'closed',
'assignee_login': 'assignee',
'created_at': '2019-08-07T00:53:51Z',
'updated_at': '2022-08-15T16:40:59Z',
'closed_at': '2022-08-15T16:40:29Z',
'labels': ['from: Google', 'position: support', 'venue: WHATWG HTML Workstream'],
},
}];
const el = await fixture(html`<chromedash-link
href="https://github.com/WebKit/standards-positions/issues/268"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/WebKit/standards-positions/issues/268"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/6458?s=48&amp;v=4">
<sl-badge size="small" variant="success">
Support
</sl-badge>
#268 Cross-Origin Embedder Policies...credentialless"
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});

function firefoxIssue() {
return {
url: 'https://github.com/mozilla/standards-positions/issues/975',
type: 'github_issue',
information: {
'url': 'https://api.github.com/repos/mozilla/standards-positions/issues/975',
'number': 975,
'title': 'The textInput event',
'user_login': 'user',
'state': 'closed',
'assignee_login': 'assignee',
'created_at': '2019-08-07T00:53:51Z',
'updated_at': '2022-08-15T16:40:59Z',
'closed_at': '2022-08-15T16:40:29Z',
'labels': ['position: neutral'],
},
};
}

it('shows Firefox position', async () => {
const featureLinks = [firefoxIssue()];
const el = await fixture(html`<chromedash-link
href="https://github.com/mozilla/standards-positions/issues/975"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/mozilla/standards-positions/issues/975"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
<sl-badge size="small" variant="neutral">
Neutral
</sl-badge>
#975 The textInput event
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});

it('shows closed if no position', async () => {
const issue = firefoxIssue();
issue.information.labels = [];
const featureLinks = [issue];
const el = await fixture(html`<chromedash-link
href="https://github.com/mozilla/standards-positions/issues/975"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/mozilla/standards-positions/issues/975"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
<sl-badge size="small" variant="neutral">
Closed
</sl-badge>
#975 The textInput event
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});

it('shows warning if open too short', async () => {
const issue = firefoxIssue();
issue.information.labels = [];
const lastWeek = new Date(Date.now() - 3 * 7 * DAY);
issue.information.state = 'open';
issue.information.created_at = lastWeek.toISOString();
const featureLinks = [issue];
const el = await fixture(html`<chromedash-link
href="https://github.com/mozilla/standards-positions/issues/975"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/mozilla/standards-positions/issues/975"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
<sl-badge size="small" variant="warning">
Opened
<sl-relative-time date="${lastWeek.toISOString()}">
on ${_dateTimeFormat.format(lastWeek)}
</sl-relative-time>
</sl-badge>
#975 The textInput event
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});

it('shows neutral if open a long time', async () => {
const issue = firefoxIssue();
issue.information.labels = [];
const twoMonths = new Date(Date.now() - 8 * 7 * DAY);
issue.information.state = 'open';
issue.information.created_at = twoMonths.toISOString();
const featureLinks = [issue];
const el = await fixture(html`<chromedash-link
href="https://github.com/mozilla/standards-positions/issues/975"
.featureLinks=${featureLinks}>Content</chromedash-link>`);
expect(el).shadowDom.to.equal(
`<a href="https://github.com/mozilla/standards-positions/issues/975"
rel="noopener noreferrer" target="_blank">
<sl-tooltip>
<div slot="content">...</div>
<sl-tag>
<img
alt="icon"
class="icon"
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
<sl-badge size="small" variant="neutral">
Opened
<sl-relative-time date="${twoMonths.toISOString()}">
on ${_dateTimeFormat.format(twoMonths)}
</sl-relative-time>
</sl-badge>
#975 The textInput event
</sl-tag>
</sl-tooltip>
</a>`,
{
ignoreChildren: ['div'],
ignoreAttributes: ['style', 'title'],
});
});
});
});
Loading