From 3a5368364314af76abd04b55184e6c0b414753ca Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Wed, 28 Jun 2023 13:13:36 -0400 Subject: [PATCH] Make annotation controls a button group (#38714) Co-authored-by: Kevin Heis --- components/lib/toggle-annotations.ts | 64 ++++++++----------- src/content-render/unified/annotate.js | 20 ++++-- stylesheets/utilities.scss | 4 -- .../rendering/__snapshots__/annotate.js.snap | 2 +- 4 files changed, 41 insertions(+), 49 deletions(-) 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/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