Skip to content

add accessibility buffer#171118

Merged
meganrogge merged 36 commits intomainfrom
merogge/xterm-acc
Jan 19, 2023
Merged

add accessibility buffer#171118
meganrogge merged 36 commits intomainfrom
merogge/xterm-acc

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

fixes #169853

@meganrogge meganrogge marked this pull request as draft January 11, 2023 23:10
@meganrogge meganrogge self-assigned this Jan 11, 2023
@meganrogge meganrogge requested a review from Tyriar January 11, 2023 23:11
@meganrogge meganrogge added this to the January 2023 milestone Jan 11, 2023
@meganrogge meganrogge marked this pull request as ready for review January 18, 2023 18:38
@Tyriar
Copy link
Copy Markdown
Contributor

Tyriar commented Jan 19, 2023

I notice mouse wheel events still skip the a11y buffer and go to the main terminal, ideally we would change that as well as when you click on it to not switch back to the main terminal view.

@meganrogge
Copy link
Copy Markdown
Collaborator Author

I notice mouse wheel events still skip the a11y buffer and go to the main terminal, ideally we would change that as well as when you click on it to not switch back to the main terminal view.

Oh, I thought that was by design as I thought ppl would be using the keyboard to navigate and mouse to escape

@meganrogge meganrogge changed the title add accessibility element to the terminal add accessibility buffer Jan 19, 2023
@Tyriar
Copy link
Copy Markdown
Contributor

Tyriar commented Jan 19, 2023

Oh, I thought that was by design as I thought ppl would be using the keyboard to navigate and mouse to escape

It was, but that was before I realized it would eventually enable keyboard caret selection. We can skip it for now as it's only on when you enable a11y mode. There are also low-vision users who may use screen readers and a mouse, so ideally it would work.

return;
}
// The viewport is undefined when this is focused, so we cannot get the cell height from that. Instead, estimate using the font.
const cellHeight = this._configHelper.getFont((this.xterm.raw as any)._core)?.charHeight || undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you use this.xterm.getFont() instead?

}
// The viewport is undefined when this is focused, so we cannot get the cell height from that. Instead, estimate using the font.
const cellHeight = this._configHelper.getFont((this.xterm.raw as any)._core)?.charHeight || undefined;
this._accessibilityBuffer.style.lineHeight = cellHeight ? (this._configurationService.getValue(TerminalSettingId.LineHeight) as number) * cellHeight + 'px' : '';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getFont should return the lineHeight as well?

@meganrogge meganrogge merged commit ef9fb8a into main Jan 19, 2023
@meganrogge meganrogge deleted the merogge/xterm-acc branch January 19, 2023 18:58
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explore plain content editable element for terminal buffer instead of navigation mode

2 participants