Skip to content

Correct lost/incorrect focus after interactions #312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wparad merged 2 commits into from
Jul 15, 2026
Merged

Correct lost/incorrect focus after interactions #312

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01dfeb4
correct focus after interactions
ahamelers Jul 15, 2026
34208b4
remove focus
ahamelers Jul 15, 2026
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
7 changes: 6 additions & 1 deletion src/openapi-explorer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ export default class OpenApiExplorer extends LitElement {
const gotoEl = this.shadowRoot.getElementById(tmpElementId);
if (gotoEl) {
gotoEl.scrollIntoView({ behavior: 'auto', block: 'start' });
gotoEl.focus();
replaceState(tmpElementId);
}
}, isExpandingNeeded ? 150 : 0);
Expand Down Expand Up @@ -459,6 +460,7 @@ export default class OpenApiExplorer extends LitElement {
const gotoEl = this.shadowRoot.getElementById(anchor.replace('#', ''));
if (gotoEl) {
gotoEl.scrollIntoView({ behavior: 'auto', block: 'start' });
gotoEl.focus();
}
}
}
Expand Down Expand Up @@ -603,19 +605,21 @@ export default class OpenApiExplorer extends LitElement {
component.expanded = true;
}
contentEl.scrollIntoView({ behavior: 'auto', block: 'start' });
contentEl.focus();

// Update Location Hash
replaceState(elementId);
newNavEl = this.shadowRoot.getElementById(`link-${elementId}`);
} else if (elementId.match('cmp--') || elementId.match('tag--') || elementId.match('overview--') || elementId.match('auth--') || elementId.match('servers--')) {
contentEl.scrollIntoView({ behavior: 'auto', block: 'start' });
contentEl.focus();

// Update Location Hash
replaceState(elementId);
newNavEl = this.shadowRoot.getElementById(`link-${elementId}`);
} else {
this.shadowRoot.getElementById('operations-root').scrollIntoView({ behavior: 'auto', block: 'start' });

this.shadowRoot.getElementById('operations-root').focus();
// Update Location Hash
replaceState(elementId);
newNavEl = this.shadowRoot.getElementById(`link-${elementId}`);
Expand All @@ -642,6 +646,7 @@ export default class OpenApiExplorer extends LitElement {
if (waitForComponentToExpand) {
setTimeout(() => newNavEl.scrollIntoView({ behavior: 'auto', block: 'center' }), 600);
}
newNavEl.focus();
}

await sleep(0);
Expand Down
1 change: 1 addition & 0 deletions src/utils/common-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function copyToClipboard(copyData, eventTarget) {
setTimeout(() => {
btnEl.innerText = getI18nText('operations.copy');
}, 5000);
btnEl.focus();

@wparad wparad Jul 15, 2026

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.

Can we quickly talk about the "why focus is lost", what is causing the bad behavior in the first place? Or said differently, what would have to change so that we wouldn't need to add this line of code?

@ahamelers ahamelers Jul 15, 2026 •
edited
Loading

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.

Focus is lost because of this:

openapi-explorer/src/utils/common-utils.js

Lines 36 to 41 in 241dc32

const textArea = document.createElement('textarea');
textArea.value = data;
textArea.style.position = 'fixed'; // avoid scrolling to bottom
document.body.appendChild(textArea);
textArea.focus();
textArea.select();

This copy function creates a stealthy textarea with what is to be copied inside of it, puts that on the page, and then focuses on it in order to select the text. Then, it deletes the textarea after this copying is performed. The focus has then disappeared from the page along with the element.

You would not need to refocus on the button, if the entire function was changed to use a different method of copying, probably something similar to this example from the clipboard API https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText#examples

@wparad wparad Jul 15, 2026

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.

It looks like we are only using it in one spot, please verify, and that's in the syntax-highlighter. And we are only passing this.content.toString() or JSON.stringify(this.content), which means we are always sending plaintext to it.

I think we should delete this method, and use the clipboard api directly in the syntax-highlighter.

@ahamelers ahamelers Jul 15, 2026

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.

Okay, do you want me to do this in this PR or in a new one? I also need to adjust some aria stuff so the 'copied' success message is provided to screen readers, if that affects your choice.

@wparad wparad Jul 15, 2026

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.

Whatever you think is best. Again, I don't know what you are envisioning so that's always a judgment call you will have to make. I can only offer advice after the fact. If decisions like this are challenging to figure out, I'm happy to help walk you through how best to think about them.

@ahamelers ahamelers Jul 15, 2026

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.

I am mainly considering the request to do one PR at a time right now. Is this one acceptable for merging so I can move on to that work?

@wparad wparad Jul 15, 2026

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.

We shouldn't add code like btnEl.focus(); just to remove it in a followup, that's the only thing that doesn't make sense. Whether you do the correct fix for the clipboard here or in another PR is up to you.

}
} catch (err) {
console.error('Unable to copy', err); // eslint-disable-line no-console
Expand Down
Loading