diff --git a/components/lib/toggle-annotations.ts b/components/lib/toggle-annotations.ts index 74c5e73cb4..27f2272608 100644 --- a/components/lib/toggle-annotations.ts +++ b/components/lib/toggle-annotations.ts @@ -1,8 +1,8 @@ import Cookies from 'components/lib/cookies' enum annotationMode { - Beside = '#annotation-beside', - Inline = '#annotation-inline', + Beside = 'beside', + Inline = 'inline', } /** @@ -11,73 +11,63 @@ enum annotationMode { * @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. * @returns The validated mode, or null if leaveNull is true and no valid mode is found. */ -function validateMode(mode?: string, leaveNull?: boolean) { - if (mode === annotationMode.Beside || mode === annotationMode.Inline || (!mode && leaveNull)) - return mode - else { - if (leaveNull) { - console.warn(`Leaving null.`) - return - } - - // default to beside - return annotationMode.Beside - } +function validateMode(mode?: string) { + if (mode === annotationMode.Beside || mode === annotationMode.Inline || !mode) return mode + // default to Beside + else return annotationMode.Beside } export default function toggleAnnotation() { - const subNavElements = Array.from(document.querySelectorAll('a.subnav-item')) - if (!subNavElements.length) return + const annotationButtons = Array.from(document.querySelectorAll('div.BtnGroup button')) + if (!annotationButtons.length) return const cookie = validateMode(Cookies.get('annotate-mode')) // will default to beside - displayAnnotationMode(subNavElements, cookie) + displayAnnotationMode(annotationButtons, cookie) // this loop adds event listeners for both the annotation buttons - for (const subnav of subNavElements) { - subnav.addEventListener('click', (evt) => { + for (const annotationBtn of annotationButtons) { + annotationBtn.addEventListener('click', (evt) => { evt.preventDefault() - // returns either: - // 1. if href is correct, the href that was passed in - // 2. if href is missing, null - const validMode = validateMode(subnav.getAttribute('href')!) - + // validate the annotation mode and set the cookie with the valid mode + const validMode = validateMode(annotationBtn.getAttribute('value')!) Cookies.set('annotate-mode', validMode!) - setActive(subNavElements, validMode) - displayAnnotationMode(subNavElements, validMode) + // set and display the annotation mode + setActive(annotationButtons, validMode) + displayAnnotationMode(annotationButtons, validMode) }) } } // 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 -function setActive(subNavElements: Array, targetMode?: string) { +function setActive(annotationButtons: Array, targetMode?: string) { const activeElements: Array = [] targetMode = validateMode(targetMode) - - subNavElements.forEach((el) => { - if (el.getAttribute('href') === targetMode) { - el.ariaCurrent = 'true' + annotationButtons.forEach((el) => { + if (el.getAttribute('value') === targetMode) { + el.ariaSelected = 'true' activeElements.push(el) - } else el.removeAttribute('aria-current') + } else el.removeAttribute('aria-selected') }) - if (!activeElements.length) throw new Error('No subnav item is active for code annotation.') + if (!activeElements.length) + throw new Error('No annotationBtn item is active for code annotation.') return activeElements } // displays the chosen annotation mode -function displayAnnotationMode(subNavItems: Array, targetMode?: string) { +function displayAnnotationMode(annotationBtnItems: Array, targetMode?: string) { if (!targetMode || targetMode === annotationMode.Beside) - subNavItems.forEach((el) => { + annotationBtnItems.forEach((el) => { el.closest('.annotate')?.classList.replace('inline', 'beside') }) else if (targetMode === annotationMode.Inline) - subNavItems.forEach((el) => { + annotationBtnItems.forEach((el) => { el.closest('.annotate')?.classList.replace('beside', 'inline') }) else throw new Error('Invalid target mode set for annotation.') - setActive(subNavItems, targetMode) + setActive(annotationBtnItems, targetMode) } diff --git a/src/content-render/unified/annotate.js b/src/content-render/unified/annotate.js index 3ee07936ef..2e041ef61c 100644 --- a/src/content-render/unified/annotate.js +++ b/src/content-render/unified/annotate.js @@ -119,23 +119,29 @@ function matchComment(lang) { function getSubnav() { const besideBtn = h( - 'a', + 'button', { - className: 'subnav-item', - href: '#annotation-beside', + name: 'annotate-display', + value: 'beside', + type: 'button', + ariaLabel: 'Display annotations beside the code sample', + className: 'BtnGroup-item btn btn-sm tooltipped tooltipped-nw', }, ['Beside'] ) const inlineBtn = h( - 'a', + 'button', { - className: 'subnav-item', - href: '#annotation-inline', + name: 'annotate-display', + value: 'inline', + type: 'button', + ariaLabel: 'Display annotations inline as comments of the code sample', + className: 'BtnGroup-item btn btn-sm tooltipped tooltipped-nw', }, ['Inline'] ) - return h('nav', { className: 'subnav mb-0 pr-2' }, [besideBtn, inlineBtn]) + return h('div', { className: 'BtnGroup' }, [besideBtn, inlineBtn]) } function template({ lang, code, rows }) { diff --git a/src/search/README.md b/src/search/README.md index 2a4504becc..136f19f11a 100644 --- a/src/search/README.md +++ b/src/search/README.md @@ -125,5 +125,5 @@ Each record represents a section of a page. Sections are derived by splitting up - 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. - Our search querying has typo tolerance. Try spelling something wrong and see what you get! - 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. -- 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). +- 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). - 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. diff --git a/src/shielding/README.md b/src/shielding/README.md new file mode 100644 index 0000000000..5fa156f537 --- /dev/null +++ b/src/shielding/README.md @@ -0,0 +1,36 @@ +# Shielding + +## Overview + +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. + +For example, there's middleware code that sees if a `GET` request +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 +case, we spot this early and return a redirect response to the same URL +without the unrecognized query string keys so that if the request follows +redirects, the eventual 200 would be normalized by a common URL so the CDN +can serve a HIT. + +Here's an in-time discussion post that summaries the *need* and much of the +recent things we've done to fortify our backend servers to avoid unnecessary +work loads: + +**[How we have fortified Docs for better resiliency and availability (June 2023)](https://github.com/github/docs-engineering/discussions/3262)** + +## How it works + +At its root, the `src/shielding/middleware/index.js` is injected into our +Express server. From there, it loads all its individual middleware handlers. + +Each middleware is one file that focuses on a single use-case. The +use-cases are borne from studying log files (CDN and Azure App Service) to +spot patterns of request abuse. + +## Notes + +- The best place to do shielding is as close to the client(s) as possible, +i.e. in the CDN or in Azure Frontdoor. Having the code in our own backend +has the advantage that it's easier to write custom business logic +along with end-to-end tests. +- Some shielding "tricks" appear in other places throughout the code +base such as controlling the 404 response for `/assets/*` URLs. diff --git a/stylesheets/utilities.scss b/stylesheets/utilities.scss index 730b33442c..8213b3402a 100644 --- a/stylesheets/utilities.scss +++ b/stylesheets/utilities.scss @@ -164,10 +164,6 @@ } } -div.annotate-header > header > nav > a { - text-decoration: none; -} - .annotate-beside, .beside .annotate-header { margin: auto; diff --git a/tests/rendering/__snapshots__/annotate.js.snap b/tests/rendering/__snapshots__/annotate.js.snap index d683ba1185..58672d1766 100644 --- a/tests/rendering/__snapshots__/annotate.js.snap +++ b/tests/rendering/__snapshots__/annotate.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`annotate renders annotations 1`] = ` -"
YAML