Skip to content

Commit d0bfd09

Browse files
authored
Merge pull request #26312 from github/repo-sync
Repo sync
2 parents 5cfedac + 3a53683 commit d0bfd09

File tree

6 files changed

+78
-50
lines changed

6 files changed

+78
-50
lines changed

components/lib/toggle-annotations.ts

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import Cookies from 'components/lib/cookies'
22

33
enum annotationMode {
4-
Beside = '#annotation-beside',
5-
Inline = '#annotation-inline',
4+
Beside = 'beside',
5+
Inline = 'inline',
66
}
77

88
/**
@@ -11,73 +11,63 @@ enum annotationMode {
1111
* @param leaveNull Alters the return value of this function. If false, the function will return the mode that was passed in or, in the case of null, the default mode. If true, the function will return null instead of using the default mode.
1212
* @returns The validated mode, or null if leaveNull is true and no valid mode is found.
1313
*/
14-
function validateMode(mode?: string, leaveNull?: boolean) {
15-
if (mode === annotationMode.Beside || mode === annotationMode.Inline || (!mode && leaveNull))
16-
return mode
17-
else {
18-
if (leaveNull) {
19-
console.warn(`Leaving null.`)
20-
return
21-
}
22-
23-
// default to beside
24-
return annotationMode.Beside
25-
}
14+
function validateMode(mode?: string) {
15+
if (mode === annotationMode.Beside || mode === annotationMode.Inline || !mode) return mode
16+
// default to Beside
17+
else return annotationMode.Beside
2618
}
2719

2820
export default function toggleAnnotation() {
29-
const subNavElements = Array.from(document.querySelectorAll('a.subnav-item'))
30-
if (!subNavElements.length) return
21+
const annotationButtons = Array.from(document.querySelectorAll('div.BtnGroup button'))
22+
if (!annotationButtons.length) return
3123

3224
const cookie = validateMode(Cookies.get('annotate-mode')) // will default to beside
33-
displayAnnotationMode(subNavElements, cookie)
25+
displayAnnotationMode(annotationButtons, cookie)
3426

3527
// this loop adds event listeners for both the annotation buttons
36-
for (const subnav of subNavElements) {
37-
subnav.addEventListener('click', (evt) => {
28+
for (const annotationBtn of annotationButtons) {
29+
annotationBtn.addEventListener('click', (evt) => {
3830
evt.preventDefault()
3931

40-
// returns either:
41-
// 1. if href is correct, the href that was passed in
42-
// 2. if href is missing, null
43-
const validMode = validateMode(subnav.getAttribute('href')!)
44-
32+
// validate the annotation mode and set the cookie with the valid mode
33+
const validMode = validateMode(annotationBtn.getAttribute('value')!)
4534
Cookies.set('annotate-mode', validMode!)
4635

47-
setActive(subNavElements, validMode)
48-
displayAnnotationMode(subNavElements, validMode)
36+
// set and display the annotation mode
37+
setActive(annotationButtons, validMode)
38+
displayAnnotationMode(annotationButtons, validMode)
4939
})
5040
}
5141
}
5242

5343
// sets the active element's aria-current, if no targetMode is set we default to "Beside", errors if it can't set either Beside or the passed in targetMode
54-
function setActive(subNavElements: Array<Element>, targetMode?: string) {
44+
function setActive(annotationButtons: Array<Element>, targetMode?: string) {
5545
const activeElements: Array<Element> = []
5646
targetMode = validateMode(targetMode)
57-
58-
subNavElements.forEach((el) => {
59-
if (el.getAttribute('href') === targetMode) {
60-
el.ariaCurrent = 'true'
47+
annotationButtons.forEach((el) => {
48+
if (el.getAttribute('value') === targetMode) {
49+
el.ariaSelected = 'true'
6150
activeElements.push(el)
62-
} else el.removeAttribute('aria-current')
51+
} else el.removeAttribute('aria-selected')
6352
})
6453

65-
if (!activeElements.length) throw new Error('No subnav item is active for code annotation.')
54+
if (!activeElements.length)
55+
throw new Error('No annotationBtn item is active for code annotation.')
6656

6757
return activeElements
6858
}
6959

7060
// displays the chosen annotation mode
71-
function displayAnnotationMode(subNavItems: Array<Element>, targetMode?: string) {
61+
function displayAnnotationMode(annotationBtnItems: Array<Element>, targetMode?: string) {
7262
if (!targetMode || targetMode === annotationMode.Beside)
73-
subNavItems.forEach((el) => {
63+
annotationBtnItems.forEach((el) => {
7464
el.closest('.annotate')?.classList.replace('inline', 'beside')
7565
})
7666
else if (targetMode === annotationMode.Inline)
77-
subNavItems.forEach((el) => {
67+
annotationBtnItems.forEach((el) => {
7868
el.closest('.annotate')?.classList.replace('beside', 'inline')
7969
})
8070
else throw new Error('Invalid target mode set for annotation.')
8171

82-
setActive(subNavItems, targetMode)
72+
setActive(annotationBtnItems, targetMode)
8373
}

src/content-render/unified/annotate.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,23 +119,29 @@ function matchComment(lang) {
119119

120120
function getSubnav() {
121121
const besideBtn = h(
122-
'a',
122+
'button',
123123
{
124-
className: 'subnav-item',
125-
href: '#annotation-beside',
124+
name: 'annotate-display',
125+
value: 'beside',
126+
type: 'button',
127+
ariaLabel: 'Display annotations beside the code sample',
128+
className: 'BtnGroup-item btn btn-sm tooltipped tooltipped-nw',
126129
},
127130
['Beside']
128131
)
129132
const inlineBtn = h(
130-
'a',
133+
'button',
131134
{
132-
className: 'subnav-item',
133-
href: '#annotation-inline',
135+
name: 'annotate-display',
136+
value: 'inline',
137+
type: 'button',
138+
ariaLabel: 'Display annotations inline as comments of the code sample',
139+
className: 'BtnGroup-item btn btn-sm tooltipped tooltipped-nw',
134140
},
135141
['Inline']
136142
)
137143

138-
return h('nav', { className: 'subnav mb-0 pr-2' }, [besideBtn, inlineBtn])
144+
return h('div', { className: 'BtnGroup' }, [besideBtn, inlineBtn])
139145
}
140146

141147
function template({ lang, code, rows }) {

src/search/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,5 @@ Each record represents a section of a page. Sections are derived by splitting up
125125
- It's not strictly necessary to set an `objectID` as the search index will create one automatically, but by creating our own we have a guarantee that subsequent invocations of this upload script will overwrite existing records instead of creating numerous duplicate records with differing IDs.
126126
- Our search querying has typo tolerance. Try spelling something wrong and see what you get!
127127
- Our search querying has lots of controls for customizing each index, so we can add weights to certain attributes and create rules like "title is more important than body", etc. But it works pretty well as-is without any configuration.
128-
- Our search querying has support for "advanced query syntax" for exact matching of quoted expressions and exclusion of words preceded by a `-` sign. This is off by default but we have it enabled in our browser client. The settings in the web interface can be overridden by the search endpoint. See [middleware/search.js]([middleware/search.js).
128+
- Our search querying has support for "advanced query syntax" for exact matching of quoted expressions and exclusion of words preceded by a `-` sign. This is off by default, but it is enabled in our browser client. The settings in the web interface can be overridden by the search endpoint. See [middleware/search.js](middleware/search.js).
129129
- When needed, the Docs Engineering team can commit updates to the search index, as long as the label `skip-index-check` is applied to the PR.

src/shielding/README.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Shielding
2+
3+
## Overview
4+
5+
Essentially code in our server that controls the prevention of "junk requests" is scripted HTTP requests to endpoints that are *not* made by regular browser users.
6+
7+
For example, there's middleware code that sees if a `GET` request
8+
comes in with a bunch of random looking query strings keys. This would cause a PASS on the CDN but would not actually matter to the rendering. In this
9+
case, we spot this early and return a redirect response to the same URL
10+
without the unrecognized query string keys so that if the request follows
11+
redirects, the eventual 200 would be normalized by a common URL so the CDN
12+
can serve a HIT.
13+
14+
Here's an in-time discussion post that summaries the *need* and much of the
15+
recent things we've done to fortify our backend servers to avoid unnecessary
16+
work loads:
17+
18+
**[How we have fortified Docs for better resiliency and availability (June 2023)](https://github.com/github/docs-engineering/discussions/3262)**
19+
20+
## How it works
21+
22+
At its root, the `src/shielding/middleware/index.js` is injected into our
23+
Express server. From there, it loads all its individual middleware handlers.
24+
25+
Each middleware is one file that focuses on a single use-case. The
26+
use-cases are borne from studying log files (CDN and Azure App Service) to
27+
spot patterns of request abuse.
28+
29+
## Notes
30+
31+
- The best place to do shielding is as close to the client(s) as possible,
32+
i.e. in the CDN or in Azure Frontdoor. Having the code in our own backend
33+
has the advantage that it's easier to write custom business logic
34+
along with end-to-end tests.
35+
- Some shielding "tricks" appear in other places throughout the code
36+
base such as controlling the 404 response for `/assets/*` URLs.

stylesheets/utilities.scss

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,6 @@
164164
}
165165
}
166166

167-
div.annotate-header > header > nav > a {
168-
text-decoration: none;
169-
}
170-
171167
.annotate-beside,
172168
.beside .annotate-header {
173169
margin: auto;

tests/rendering/__snapshots__/annotate.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`annotate renders annotations 1`] = `
4-
"<div class="annotate beside"><div class="annotate-header"><header class="d-flex flex-items-center flex-justify-between p-2 text-small rounded-top-1 border-top border-left border-right"><span class="flex-1">YAML</span><nav class="subnav mb-0 pr-2"><a class="subnav-item" href="#annotation-beside">Beside</a><a class="subnav-item" href="#annotation-inline">Inline</a></nav><button class="js-btn-copy btn btn-sm tooltipped tooltipped-nw" aria-label="Copy code to clipboard" data-clipboard="1746955726"><svg version="1.1" width="16" height="16" viewBox="0 0 16 16" class="octicon octicon-copy" aria-hidden="true"><path d="M0 6.75C0 5.784.784 5 1.75 5h1.5a.75.75 0 0 1 0 1.5h-1.5a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-1.5a.75.75 0 0 1 1.5 0v1.5A1.75 1.75 0 0 1 9.25 16h-7.5A1.75 1.75 0 0 1 0 14.25Z"></path><path d="M5 1.75C5 .784 5.784 0 6.75 0h7.5C15.216 0 16 .784 16 1.75v7.5A1.75 1.75 0 0 1 14.25 11h-7.5A1.75 1.75 0 0 1 5 9.25Zm1.75-.25a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-7.5a.25.25 0 0 0-.25-.25Z"></path></svg></button><pre hidden data-clipboard="1746955726"># The name of the workflow as it will appear in the "Actions" tab of the GitHub repository.
4+
"<div class="annotate beside"><div class="annotate-header"><header class="d-flex flex-items-center flex-justify-between p-2 text-small rounded-top-1 border-top border-left border-right"><span class="flex-1">YAML</span><div class="BtnGroup"><button name="annotate-display" value="beside" type="button" aria-label="Display annotations beside the code sample" class="BtnGroup-item btn btn-sm tooltipped tooltipped-nw">Beside</button><button name="annotate-display" value="inline" type="button" aria-label="Display annotations inline as comments of the code sample" class="BtnGroup-item btn btn-sm tooltipped tooltipped-nw">Inline</button></div><button class="js-btn-copy btn btn-sm tooltipped tooltipped-nw" aria-label="Copy code to clipboard" data-clipboard="1746955726"><svg version="1.1" width="16" height="16" viewBox="0 0 16 16" class="octicon octicon-copy" aria-hidden="true"><path d="M0 6.75C0 5.784.784 5 1.75 5h1.5a.75.75 0 0 1 0 1.5h-1.5a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-1.5a.75.75 0 0 1 1.5 0v1.5A1.75 1.75 0 0 1 9.25 16h-7.5A1.75 1.75 0 0 1 0 14.25Z"></path><path d="M5 1.75C5 .784 5.784 0 6.75 0h7.5C15.216 0 16 .784 16 1.75v7.5A1.75 1.75 0 0 1 14.25 11h-7.5A1.75 1.75 0 0 1 5 9.25Zm1.75-.25a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h7.5a.25.25 0 0 0 .25-.25v-7.5a.25.25 0 0 0-.25-.25Z"></path></svg></button><pre hidden data-clipboard="1746955726"># The name of the workflow as it will appear in the "Actions" tab of the GitHub repository.
55
name: Post welcome comment
66
77
# Add the \`pull_request\` event, so that the workflow runs automatically

0 commit comments

Comments
 (0)