Skip to content
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

Jump to bottom

Accessibility: jumps in review #5143

Closed
codeofdusk opened this issue Mar 27, 2020 · 6 comments · Fixed by #5196
Closed

Accessibility: jumps in review #5143

codeofdusk opened this issue Mar 27, 2020 · 6 comments · Fixed by #5196
Assignees
@carlos-zamora
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@codeofdusk
Copy link
Contributor

Environment

Windows build number: [run [Environment]::OSVersion for powershell, or ver for cmd]: 1909 (18363.720)

Windows Terminal version (if applicable): Terminal as of 5bcf0fc.

Any other software? NVDA, Narrator.

Steps to reproduce

  1. type:
echo 1
echo 2
echo 3
  1. Using NVDA with Windows Terminal support or Narrator, review a few previous lines of output. You should see the prompt, then 3, 2, 1.
  2. wait about three seconds.
  3. Try to review previous line again.

Expected behavior

The screen reader keeps its position, reporting "2" for example if the last-reviewed line was "3".

Actual behavior

The screen reader reads the prompt again, then starts back over with 3, almost as if the cursor bounced back.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 27, 2020
@codeofdusk
Copy link
Contributor Author

Cc @carlos-zamora.

@codeofdusk
Copy link
Contributor Author

This cursor bouncing does not occur in open console (that works as expected).

@carlos-zamora carlos-zamora added Area-Accessibility Issues related to accessibility Priority-0 Bugs that we consider release-blocking/recall-class (P0) and removed Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Mar 27, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Mar 27, 2020
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Mar 27, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 27, 2020
@carlos-zamora carlos-zamora added v1-Scrubbed Priority-1 A description (P1) and removed Priority-0 Bugs that we consider release-blocking/recall-class (P0) labels Mar 27, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Mar 27, 2020
@carlos-zamora carlos-zamora self-assigned this Mar 27, 2020
@carlos-zamora
Copy link
Member

I tested this with NVDA and Narrator. NVDA 2020.1 does not reproduce the above example, but Narrator does.

After disabling cursor blinking, scan mode on Narrator is no longer interrupted. After #5135 goes in, I may be able to leverage the new winrt event to only fire a CursorChanged event on cursor movements, instead of every time we render the cursor. This should be a lot less noisy.

@codeofdusk codeofdusk mentioned this issue Mar 29, 2020
Merged
@codeofdusk
Copy link
Contributor Author

After #5135 goes in, I may be able to leverage the new winrt event to only fire a CursorChanged event on cursor movements, instead of every time we render the cursor. This should be a lot less noisy.

Now that #5135 has been merged, does this still seem possible?

@carlos-zamora
Copy link
Member

After #5135 goes in, I may be able to leverage the new winrt event to only fire a CursorChanged event on cursor movements, instead of every time we render the cursor. This should be a lot less noisy.

Now that #5135 has been merged, does this still seem possible?

Yeah. Working on it now. Hoping to get a PR in by the end of the day or tomorrow 😊

@carlos-zamora carlos-zamora mentioned this issue Mar 31, 2020
Merged
1 task
@ghost ghost added the In-PR This issue has a related PR label Mar 31, 2020
@ghost ghost closed this as completed in #5196 Apr 1, 2020
ghost pushed a commit that referenced this issue Apr 1, 2020
@carlos-zamora
Reduce CursorChanged Events for Accessibility (#5196)
f8227e6 
## Summary of the Pull Request
Reduce the number of times we dispatch a cursor changed event. We were firing it every time the renderer had to do anything related to the cursor. Unfortunately, blinking the cursor triggered this behavior. Now we just check if the position has changed.

## PR Checklist
* [X] Closes #5143


## Validation Steps Performed
Verified using Narrator
Also verified #3791 still works right
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 1, 2020
DHowett-MSFT pushed a commit that referenced this issue Apr 14, 2020
@carlos-zamora @DHowett
Reduce CursorChanged Events for Accessibility (#5196)
8de952c 
## Summary of the Pull Request
Reduce the number of times we dispatch a cursor changed event. We were firing it every time the renderer had to do anything related to the cursor. Unfortunately, blinking the cursor triggered this behavior. Now we just check if the position has changed.

## PR Checklist
* [X] Closes #5143


## Validation Steps Performed
Verified using Narrator
Also verified #3791 still works right
@ghost
Copy link

ghost commented Apr 22, 2020

🎉This issue was addressed in #5196, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Reduce CursorChanged Events for Accessibility microsoft/terminal
3 participants
@codeofdusk @carlos-zamora @DHowett-MSFT