Skip to content

Commit 9d0786f

Browse files
authored
Fancy links for GH issues representing external reviews (#3749)
Links report the time since they were opened (yellow for too-young issues) or the external reviewer's position, with color codes for how favorable the outcome was.
1 parent 9617877 commit 9d0786f

File tree

5 files changed

+376
-9
lines changed

5 files changed

+376
-9
lines changed

client-src/elements/chromedash-link.js

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import {css, html, LitElement} from 'lit';
44
import {ifDefined} from 'lit/directives/if-defined.js';
55
import {SHARED_STYLES} from '../css/shared-css';
6+
import {ExternalReviewer} from './external-reviewers';
67

78
// LINK_TYPES should be consistent with the server link_helpers.py
89
const LINK_TYPE_CHROMIUM_BUG = 'chromium_bug';
@@ -97,7 +98,7 @@ function enhanceGithubIssueLink(featureLink, text) {
9798
}
9899
const information = featureLink.information;
99100
const assignee = information.assignee_login;
100-
const createdAt = information.created_at;
101+
const createdAt = new Date(information.created_at);
101102
const closedAt = information.closed_at;
102103
const updatedAt = information.updated_at;
103104
const state = information.state;
@@ -109,6 +110,43 @@ function enhanceGithubIssueLink(featureLink, text) {
109110
const typePath = featureLink.url.split('/').slice(-2)[0];
110111
const type = typePath === 'issues' ? 'Issue' : typePath === 'pull' ? 'Pull Request' : typePath;
111112

113+
// If this issue is an external review of the feature, find the summary description.
114+
const externalReviewer = ExternalReviewer.get(repo);
115+
let stateDescription = undefined;
116+
let stateVariant = undefined;
117+
if (externalReviewer) {
118+
for (const label of information.labels) {
119+
const labelInfo = externalReviewer.label(label);
120+
if (labelInfo) {
121+
({description: stateDescription, variant: stateVariant} = labelInfo);
122+
break;
123+
}
124+
}
125+
}
126+
127+
if (stateVariant === undefined) {
128+
if (state === 'open') {
129+
const age = Date.now() - createdAt.getTime();
130+
stateDescription = html`Opened <sl-relative-time date=${createdAt.toISOString()}>on ${_dateTimeFormat.format(createdAt)}</sl-relative-time>`;
131+
const week = 7 * 24 * 60 * 60 * 1000;
132+
stateVariant = 'success';
133+
if (externalReviewer) {
134+
// If this is an issue asking for external review, having it filed too recently is a warning
135+
// sign, which we'll indicate using the tag's color.
136+
if (age < 4 * week) {
137+
stateVariant = 'warning';
138+
} else {
139+
// Still only neutral if the reviewer hasn't given a position yet.
140+
stateVariant = 'neutral';
141+
}
142+
}
143+
} else {
144+
console.assert(state === 'closed');
145+
stateDescription = 'Closed';
146+
stateVariant = 'neutral';
147+
}
148+
}
149+
112150
if (!text) {
113151
text = title;
114152
}
@@ -169,8 +207,8 @@ function enhanceGithubIssueLink(featureLink, text) {
169207
<sl-tooltip style="--sl-tooltip-arrow-size: 0;--max-width: 50vw;">
170208
<div slot="content">${renderTooltipContent()}</div>
171209
<sl-tag>
172-
<img src="https://docs.github.com/assets/cb-600/images/site/favicon.png" alt="icon" class="icon" />
173-
<sl-badge size="small" variant="${state === 'open' ? 'success' : 'neutral'}">${state}</sl-badge>
210+
<img src=${externalReviewer?.icon ?? 'https://docs.github.com/assets/cb-600/images/site/favicon.png'} alt="icon" class="icon" />
211+
<sl-badge size="small" variant=${stateVariant}>${stateDescription}</sl-badge>
174212
${_formatLongText(`#${number} ` + text)}
175213
</sl-tag>
176214
</sl-tooltip>
@@ -337,8 +375,8 @@ export class ChromedashLink extends LitElement {
337375
}
338376
339377
sl-badge::part(base) {
340-
height: 14px;
341-
padding: 4px;
378+
display: inline;
379+
padding: 0 4px;
342380
border-width: 0;
343381
text-transform: capitalize;
344382
font-weight: 400;
@@ -362,6 +400,10 @@ export class ChromedashLink extends LitElement {
362400
background-color: rgb(209,211,213);
363401
}
364402
403+
sl-relative-time {
404+
margin: 0;
405+
}
406+
365407
.icon {
366408
display: block;
367409
width: 12px;

client-src/elements/chromedash-link_test.js

Lines changed: 250 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {html} from 'lit';
33
import './chromedash-link';
44
import {_dateTimeFormat} from './chromedash-link';
55

6+
const DAY = 24 * 60 * 60 * 1000;
7+
68
it('shows a plain link for non-cached links', async () => {
79
const el = await fixture(html`<chromedash-link
810
href="https://github.com/GoogleChrome/chromium-dashboard/issues/3007"
@@ -27,8 +29,11 @@ it('wraps the text in sl-tag when requested', async () => {
2729
});
2830

2931
describe('Github issue links', () => {
32+
// Get the 'information' object by running
33+
// `curl -L https://api.github.com/repos/OWNER/REPO/issues/ISSUE_NUMBER`
34+
3035
it('shows tooltip, open state, and title', async () => {
31-
const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000);
36+
const yesterday = new Date(Date.now() - DAY);
3237
const featureLinks = [{
3338
url: 'https://github.com/GoogleChrome/chromium-dashboard/issues/3007',
3439
type: 'github_issue',
@@ -94,7 +99,10 @@ describe('Github issue links', () => {
9499
class="icon"
95100
src="https://docs.github.com/assets/cb-600/images/site/favicon.png">
96101
<sl-badge size="small" variant="success">
97-
open
102+
Opened
103+
<sl-relative-time date="${yesterday.toISOString()}">
104+
on ${_dateTimeFormat.format(yesterday)}
105+
</sl-relative-time>
98106
</sl-badge>
99107
#3007 Issue Title
100108
</sl-tag>
@@ -106,7 +114,7 @@ describe('Github issue links', () => {
106114
});
107115

108116
it('shows closed state and title', async () => {
109-
const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000);
117+
const yesterday = new Date(Date.now() - DAY);
110118
const featureLinks = [{
111119
url: 'https://github.com/GoogleChrome/chromium-dashboard/issues/3007',
112120
type: 'github_issue',
@@ -137,7 +145,7 @@ describe('Github issue links', () => {
137145
class="icon"
138146
src="https://docs.github.com/assets/cb-600/images/site/favicon.png">
139147
<sl-badge size="small" variant="neutral">
140-
closed
148+
Closed
141149
</sl-badge>
142150
#3007 Issue Title
143151
</sl-tag>
@@ -148,4 +156,242 @@ describe('Github issue links', () => {
148156
ignoreAttributes: ['style', 'title'],
149157
});
150158
});
159+
160+
describe('external reviewers', () => {
161+
it('shows TAG position', async () => {
162+
const featureLinks = [{
163+
url: 'https://github.com/w3ctag/design-reviews/issues/400',
164+
type: 'github_issue',
165+
information: {
166+
'url': 'https://api.github.com/repos/w3ctag/design-reviews/issues/400',
167+
'number': 400,
168+
'title': 'Font Table Access API',
169+
'user_login': 'user',
170+
'state': 'closed',
171+
'assignee_login': 'assignee',
172+
'created_at': '2019-08-07T00:53:51Z',
173+
'updated_at': '2022-08-15T16:40:59Z',
174+
'closed_at': '2022-08-15T16:40:29Z',
175+
'labels': ['Needs a venue', 'Provenance: Fugu', 'Resolution: unsatisfied',
176+
'Review type: CG early review', 'Topic: fonts'],
177+
},
178+
}];
179+
const el = await fixture(html`<chromedash-link
180+
href="https://github.com/w3ctag/design-reviews/issues/400"
181+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
182+
expect(el).shadowDom.to.equal(
183+
`<a href="https://github.com/w3ctag/design-reviews/issues/400"
184+
rel="noopener noreferrer" target="_blank">
185+
<sl-tooltip>
186+
<div slot="content">...</div>
187+
<sl-tag>
188+
<img
189+
alt="icon"
190+
class="icon"
191+
src="https://avatars.githubusercontent.com/u/3874462?s=48&amp;v=4">
192+
<sl-badge size="small" variant="danger">
193+
Unsatisfied
194+
</sl-badge>
195+
#400 Font Table Access API
196+
</sl-tag>
197+
</sl-tooltip>
198+
</a>`,
199+
{
200+
ignoreChildren: ['div'],
201+
ignoreAttributes: ['style', 'title'],
202+
});
203+
});
204+
205+
it('shows WebKit position', async () => {
206+
const featureLinks = [{
207+
url: 'https://github.com/WebKit/standards-positions/issues/268',
208+
type: 'github_issue',
209+
information: {
210+
'url': 'https://api.github.com/repos/WebKit/standards-positions/issues/268',
211+
'number': 268,
212+
'title': 'Cross-Origin Embedder Policies - "credentialless"',
213+
'user_login': 'user',
214+
'state': 'closed',
215+
'assignee_login': 'assignee',
216+
'created_at': '2019-08-07T00:53:51Z',
217+
'updated_at': '2022-08-15T16:40:59Z',
218+
'closed_at': '2022-08-15T16:40:29Z',
219+
'labels': ['from: Google', 'position: support', 'venue: WHATWG HTML Workstream'],
220+
},
221+
}];
222+
const el = await fixture(html`<chromedash-link
223+
href="https://github.com/WebKit/standards-positions/issues/268"
224+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
225+
expect(el).shadowDom.to.equal(
226+
`<a href="https://github.com/WebKit/standards-positions/issues/268"
227+
rel="noopener noreferrer" target="_blank">
228+
<sl-tooltip>
229+
<div slot="content">...</div>
230+
<sl-tag>
231+
<img
232+
alt="icon"
233+
class="icon"
234+
src="https://avatars.githubusercontent.com/u/6458?s=48&amp;v=4">
235+
<sl-badge size="small" variant="success">
236+
Support
237+
</sl-badge>
238+
#268 Cross-Origin Embedder Policies...credentialless"
239+
</sl-tag>
240+
</sl-tooltip>
241+
</a>`,
242+
{
243+
ignoreChildren: ['div'],
244+
ignoreAttributes: ['style', 'title'],
245+
});
246+
});
247+
248+
function firefoxIssue() {
249+
return {
250+
url: 'https://github.com/mozilla/standards-positions/issues/975',
251+
type: 'github_issue',
252+
information: {
253+
'url': 'https://api.github.com/repos/mozilla/standards-positions/issues/975',
254+
'number': 975,
255+
'title': 'The textInput event',
256+
'user_login': 'user',
257+
'state': 'closed',
258+
'assignee_login': 'assignee',
259+
'created_at': '2019-08-07T00:53:51Z',
260+
'updated_at': '2022-08-15T16:40:59Z',
261+
'closed_at': '2022-08-15T16:40:29Z',
262+
'labels': ['position: neutral'],
263+
},
264+
};
265+
}
266+
267+
it('shows Firefox position', async () => {
268+
const featureLinks = [firefoxIssue()];
269+
const el = await fixture(html`<chromedash-link
270+
href="https://github.com/mozilla/standards-positions/issues/975"
271+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
272+
expect(el).shadowDom.to.equal(
273+
`<a href="https://github.com/mozilla/standards-positions/issues/975"
274+
rel="noopener noreferrer" target="_blank">
275+
<sl-tooltip>
276+
<div slot="content">...</div>
277+
<sl-tag>
278+
<img
279+
alt="icon"
280+
class="icon"
281+
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
282+
<sl-badge size="small" variant="neutral">
283+
Neutral
284+
</sl-badge>
285+
#975 The textInput event
286+
</sl-tag>
287+
</sl-tooltip>
288+
</a>`,
289+
{
290+
ignoreChildren: ['div'],
291+
ignoreAttributes: ['style', 'title'],
292+
});
293+
});
294+
295+
it('shows closed if no position', async () => {
296+
const issue = firefoxIssue();
297+
issue.information.labels = [];
298+
const featureLinks = [issue];
299+
const el = await fixture(html`<chromedash-link
300+
href="https://github.com/mozilla/standards-positions/issues/975"
301+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
302+
expect(el).shadowDom.to.equal(
303+
`<a href="https://github.com/mozilla/standards-positions/issues/975"
304+
rel="noopener noreferrer" target="_blank">
305+
<sl-tooltip>
306+
<div slot="content">...</div>
307+
<sl-tag>
308+
<img
309+
alt="icon"
310+
class="icon"
311+
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
312+
<sl-badge size="small" variant="neutral">
313+
Closed
314+
</sl-badge>
315+
#975 The textInput event
316+
</sl-tag>
317+
</sl-tooltip>
318+
</a>`,
319+
{
320+
ignoreChildren: ['div'],
321+
ignoreAttributes: ['style', 'title'],
322+
});
323+
});
324+
325+
it('shows warning if open too short', async () => {
326+
const issue = firefoxIssue();
327+
issue.information.labels = [];
328+
const lastWeek = new Date(Date.now() - 3 * 7 * DAY);
329+
issue.information.state = 'open';
330+
issue.information.created_at = lastWeek.toISOString();
331+
const featureLinks = [issue];
332+
const el = await fixture(html`<chromedash-link
333+
href="https://github.com/mozilla/standards-positions/issues/975"
334+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
335+
expect(el).shadowDom.to.equal(
336+
`<a href="https://github.com/mozilla/standards-positions/issues/975"
337+
rel="noopener noreferrer" target="_blank">
338+
<sl-tooltip>
339+
<div slot="content">...</div>
340+
<sl-tag>
341+
<img
342+
alt="icon"
343+
class="icon"
344+
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
345+
<sl-badge size="small" variant="warning">
346+
Opened
347+
<sl-relative-time date="${lastWeek.toISOString()}">
348+
on ${_dateTimeFormat.format(lastWeek)}
349+
</sl-relative-time>
350+
</sl-badge>
351+
#975 The textInput event
352+
</sl-tag>
353+
</sl-tooltip>
354+
</a>`,
355+
{
356+
ignoreChildren: ['div'],
357+
ignoreAttributes: ['style', 'title'],
358+
});
359+
});
360+
361+
it('shows neutral if open a long time', async () => {
362+
const issue = firefoxIssue();
363+
issue.information.labels = [];
364+
const twoMonths = new Date(Date.now() - 8 * 7 * DAY);
365+
issue.information.state = 'open';
366+
issue.information.created_at = twoMonths.toISOString();
367+
const featureLinks = [issue];
368+
const el = await fixture(html`<chromedash-link
369+
href="https://github.com/mozilla/standards-positions/issues/975"
370+
.featureLinks=${featureLinks}>Content</chromedash-link>`);
371+
expect(el).shadowDom.to.equal(
372+
`<a href="https://github.com/mozilla/standards-positions/issues/975"
373+
rel="noopener noreferrer" target="_blank">
374+
<sl-tooltip>
375+
<div slot="content">...</div>
376+
<sl-tag>
377+
<img
378+
alt="icon"
379+
class="icon"
380+
src="https://avatars.githubusercontent.com/u/131524?s=48&amp;v=4">
381+
<sl-badge size="small" variant="neutral">
382+
Opened
383+
<sl-relative-time date="${twoMonths.toISOString()}">
384+
on ${_dateTimeFormat.format(twoMonths)}
385+
</sl-relative-time>
386+
</sl-badge>
387+
#975 The textInput event
388+
</sl-tag>
389+
</sl-tooltip>
390+
</a>`,
391+
{
392+
ignoreChildren: ['div'],
393+
ignoreAttributes: ['style', 'title'],
394+
});
395+
});
396+
});
151397
});

0 commit comments

Comments
 (0)