Skip to content
Open
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
6 changes: 3 additions & 3 deletions src/components/api-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,14 +659,14 @@ export default class ApiRequest extends LitElement {
</div>`
: html`
<div class="tab-content col m-markdown" style="flex:1; display:${this.activeResponseTab === 'response' ? 'flex' : 'none'};" >
<syntax-highlighter style="min-height: 60px" mime-type="${this.responseContentType}" .content="${this.responseText}"/>
<syntax-highlighter style="min-height: 60px" mime-type="${this.responseContentType}" .content="${this.responseText}" .label="Response text"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost forgot, all of these labels need to be i18n keys instead of hard coded text because we support multiple languages.

</div>`
}
<div class="tab-content col m-markdown" style="flex:1;display:${this.activeResponseTab === 'headers' ? 'flex' : 'none'};" >
<syntax-highlighter style="min-height: 60px" language="http" .content="${this.responseHeaders}"/>
<syntax-highlighter style="min-height: 60px" language="http" .content="${this.responseHeaders}" .label="Response headers"/>
</div>
<div class="tab-content m-markdown col" style="flex:1;display:${this.activeResponseTab === 'curl' ? 'flex' : 'none'};">
<syntax-highlighter style="min-height: 60px" language="shell" .content="${curlSyntax.trim()}"/>
<syntax-highlighter style="min-height: 60px" language="shell" .content="${curlSyntax.trim()}" .label="Request example"/>
</div>
</div>`;
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/api-response.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export default class ApiResponse extends LitElement {
? html`
${mimeRespDetails.examples[0].exampleSummary && mimeRespDetails.examples[0].exampleSummary.length > 80 ? html`<div style="padding: 4px 0"> ${mimeRespDetails.examples[0].exampleSummary} </div>` : ''}
${mimeRespDetails.examples[0].exampleDescription ? html`<div class="m-markdown-small" style="padding: 4px 0"> ${unsafeHTML(toMarkdown(mimeRespDetails.examples[0].exampleDescription || ''))} </div>` : ''}
<syntax-highlighter class='example-panel generic-tree pad-top-8' mime-type="${mimeRespDetails.examples[0].exampleType}" .content="${mimeRespDetails.examples[0].exampleValue}"/>`
<syntax-highlighter class='example-panel generic-tree pad-top-8' mime-type="${mimeRespDetails.examples[0].exampleType}" .content="${mimeRespDetails.examples[0].exampleValue}" .label="Response example"/>`
: html`
<span class = 'example-panel generic-tree ${this.renderStyle === 'read' ? 'border pad-8-16' : 'border-top pad-top-8'}'>
<select aria-label='response body example' @change='${(e) => this.onSelectExample(e)}'>
Expand All @@ -287,7 +287,7 @@ export default class ApiResponse extends LitElement {
<div class="example" data-example = '${v.exampleId}' style = "display: ${v.exampleId === mimeRespDetails.selectedExample ? 'block' : 'none'}">
${v.exampleSummary && v.exampleSummary.length > 80 ? html`<div style="padding: 4px 0"> ${v.exampleSummary} </div>` : ''}
${v.exampleDescription && v.exampleDescription !== v.exampleSummary ? html`<div class="m-markdown-small" style="padding: 4px 0"> ${unsafeHTML(toMarkdown(v.exampleDescription || ''))} </div>` : ''}
<syntax-highlighter mime-type="${v.exampleType}" .content="${v.exampleValue}"/>
<syntax-highlighter mime-type="${v.exampleType}" .content="${v.exampleValue}" .label="Response example"/>
</div>
`)}
</span>
Expand Down
14 changes: 9 additions & 5 deletions src/components/syntax-highlighter.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class SyntaxHighlighter extends LitElement {
static get properties() {
return {
content: { type: Object },
label: { type: String },
language: { type: String, attribute: 'language' },
mimeType: { type: String, attribute: 'mime-type' },
};
Expand Down Expand Up @@ -96,7 +97,7 @@ class SyntaxHighlighter extends LitElement {
}

render() {
return this.renderCopyWrapper(this.renderHighlight());
return this.renderCopyWrapper(this.renderHighlight(), this.label.toLowerCase());
}

/**
Expand All @@ -105,6 +106,7 @@ class SyntaxHighlighter extends LitElement {
*/
renderHighlight() {
const lang = this.detectLanguage();
const label = this.label.toLowerCase();
const grammar = Prism.languages[lang];

if (typeof this.content !== 'string') {
Expand All @@ -114,20 +116,22 @@ class SyntaxHighlighter extends LitElement {
const stringContent = this.content?.toString() || '';
const increasedSpaceContent = lang !== 'python' && lang !== 'yaml' && lang !== 'toml' ? stringContent.split('\n').map(line => line.replace(/^\s{2}/g, ' ')).join('\n') : stringContent;
return grammar
? html`<pre><code>${unsafeHTML(Prism.highlight(increasedSpaceContent, grammar, lang))}</code></pre>`
: html`<pre>${increasedSpaceContent}</pre>`;
? html`<pre tabindex="0" role="region" aria-label="${label}"><code>${unsafeHTML(Prism.highlight(increasedSpaceContent, grammar, lang))}</code></pre>`
: html`<pre tabindex="0" role="region" aria-label="${label}">${increasedSpaceContent}</pre>`;
}

/**
* Render a copy-to-clipboard button.
* @param {*} content Content
* @returns Content
*/
renderCopyWrapper(content) {
renderCopyWrapper(content, label) {
return html`<div class="fs-exclude ph-no-capture" data-hj-suppress data-sl="mask" style="min-height: 2rem;">
<button
class="m-btn outline-primary toolbar-copy-btn"
@click='${this.copyToClipboard}'
@click='${this.copyToClipboard}'
aria-label="${getI18nText('operations.copy')} ${label}"
aria-live="polite"
part="btn btn-fill btn-copy">${getI18nText('operations.copy')}</button>
${content}
</div>`;
Expand Down
2 changes: 1 addition & 1 deletion src/templates/code-samples-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function codeSamplesTemplate(xCodeSamples) {
const fullSource = sanitizedSource.join('\n');
return html`
<div class="tab-content m-markdown code-sample-wrapper" style= "display:${i === 0 ? 'block' : 'none'}" data-tab = '${v.lang}${i}'>
<syntax-highlighter language="${v.lang}" .content="${fullSource}"/>
<syntax-highlighter language="${v.lang}" .content="${fullSource}" .label="Code sample"/>
</div>`;
})
}
Expand Down
17 changes: 6 additions & 11 deletions src/utils/common-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export function sleep(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

export function copyToClipboard(copyData, eventTarget) {
export async function copyToClipboard(copyData, eventTarget) {
const btnEl = eventTarget?.target;
// In lots of places we have more than a couple of spaces for <pre> display purposes, we remove those extra spaces here.
let data = copyData?.trim().replace(/\s{8}/g, ' ');
try {
Expand All @@ -32,26 +33,20 @@ export function copyToClipboard(copyData, eventTarget) {
} catch (error) {
// Ignore non JSON text;
}

const textArea = document.createElement('textarea');
textArea.value = data;
textArea.style.position = 'fixed'; // avoid scrolling to bottom
document.body.appendChild(textArea);
textArea.focus();
textArea.select();
try {
document.execCommand('copy');
const btnEl = eventTarget?.target;
await window.navigator.clipboard.writeText(data);
if (btnEl) {
const label = btnEl.getAttribute('aria-label');
btnEl.innerText = getI18nText('operations.copied');
btnEl.setAttribute('aria-label', getI18nText('operations.copied'));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the correct thing to do for a screen reader? Setting the aria-label and changing it? That feels wrong to me, but I don't really understand how aria-labels work. What's your thinking here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit complicated because the button itself contains the status update text for the success ('copied'). We want to let users know the 'copied' success status, so there's an aria-live region on the button. But screen readers will never read the actual content of the button if aria-label is also set, they will always read that instead. So we have to update the aria-label as well for the status to be announced.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refuse to believe this is right way to solve this problem for screen readers. Why would a screen reader announce the button label change?

Multiple llms suggested to me to have a separate hidden div with the text that can be targeted for update instead of this here. Because actually I worry that since we are updating the button text twice which means that it could actually display both the "copied" message and then again the "label" message which not correct.

Is that accurate, IDK, but that feels better than this implementation.

setTimeout(() => {
btnEl.innerText = getI18nText('operations.copy');
btnEl.setAttribute('aria-label', label);
}, 5000);
}
} catch (err) {
console.error('Unable to copy', err); // eslint-disable-line no-console
}
document.body.removeChild(textArea);
}

export function getBaseUrlFromUrl(url) {
Expand Down
Loading