Enhanced Accessibility Signals with Advanced User and Modality-Specific Delays

[ramoncorominas]

fixes #204257

Summary of the issue

The current accessibility signals implementation for "line features" (those that play while navigating code) has several issues that this PR aims to address:

1. Accessibility signals disrupt the natural reading of the content. Screen reader users hear sounds and announcements every time the cursor position changes. Although this behavior can be slightly mitigated by enabling the accessibility.signals.debouncePositionChanges' setting, it only introduces a short, non-configurable delay of 300 ms. This delay may be more or less useful for users who type quickly or are accustomed to reading content rapidly, but not for less advanced users or those who wish to navigate the content more slowly for better understanding or spell-checking. For instance, a variable named "user" containing a warning will be read character by character as "u warning s warning e warning r warning" unless the user moves very quickly to allow debouncing, making it difficult to comprehend the variable name. Additionally, a "warning" sound will play simultaneously, further complicating the speech synthesis understanding.
2. The current implementation doesn't consider the distinct nature of audio and speech feedback. Sound accessibility signals (formerly Audio Cues) play sound effects that are inherently different from speech and can even be played on a separate audio channel. Speech announcements, however, are played using ARIA live regions, and therefore cannot be separated/distinguished from the normal speech of the screen reader. This means that while sound signals playing simultaneously (or almost simultaneously) with normal reading might be less disruptive, speech signals will significantly interfere with code comprehension, especially when trying to carefully read the content within the line.
3. Debouncing position changes creates synchronization issues. The current debouncing implementation merely delays the moment when the features are detected for a specific position, so the detection (and its corresponding signals) will be played with a fixed delay of 300 ms after the position has changed, but the detection is still based on the original position. Therefore, if the cursor changes to a new position during this interval, it can happen that the pending detection fires a signal that will be played out of sync with the real, current position. For example, if the user is on a line with a breakpoint and moves quickly to another line without special features, the breakpoint sound and the corresponding announcement will be heard on the new line, out of sync with the trigger that originated the signal when the old position features were detected on the previous line.
4. The current implementation doesn't allow different delays for different types of line features. The feedback for "critical" features such as errors or warnings should be provided as quickly as possible (with enough delay to avoid interferences with reading), while "informational" features such as breakpoints or folded areas (which also apply to the full line and therefore will be read on every character), should have longer delays to allow reading the line in a more comfortable way.
5. The current debouncing doesn't consider different navigation styles. From a screen reader user's perspective, reading code consists of two distinct styles: quickly skimming from line to line to get an overall idea of the code, and carefully reading the contents of a line character by character or word by word, also paying attention to syntax and punctuation. These distinct styles require different delays for the feedback signals: while the "skimming" style requires shorter delays for immediate feedback, the "content" style will need longer delays to avoid interfering with the detailed reading.

Enhanced Solution Strategy

This pull request significantly enhances the accessibility signals feature in VS Code, offering a more intuitive and user-focused experience. The final implementation effectively manages various signal modalities, including audio and speech, each with unique delays. These delays are further refined based on user navigation patterns, such as skimming through lines versus reading content within each line. This dual-level customization significantly improves the flexibility and adaptability of signal handling.

Key developments in this feature include:

1. Modality Support: The SignalModality enum type has been introduced, which enables the specification of desired modalities for each call to cater to their distinct nature. This foundational change allows for the separation and individual handling of different modalities, such as sound or speech, based on various conditions like triggers or settings. This separation is necessary to facilitate the rest of the additions in the pull request.
2. Modality-Specific Delays: Each signal modality can now have its own distinct delay. This enhancement allows for the nuanced handling of different modalities, catering to their unique requirements.
3. Feature-Specific Delays: A BaseLineFeature class was introduced that supports feature-specific delays. This update allows unique delays for different types of features, providing a more tailored user experience.
4. User-Action Dependent Delays: The system now adjusts delays based on user navigation behavior. When a user navigates within the same line, longer delays are applied to minimize interference with screen reader output. Conversely, shorter delays are implemented for line changes, providing timely feedback for users who are skimming through the program.
5. User-Configurable Delays: All delays are now adjustable, providing users the flexibility to fine-tune based on their reading speed or skill level.

These changes collectively provide a more responsive and user-centric accessibility signals feature in VS Code, enhancing the overall user experience.

Testing the Feature

1. Open a test file containing code.
2. Enhance the test environment by adding breakpoints, folding code blocks, and intentionally introducing bugs to generate errors and/or warnings in the editor.
3. Navigate through the code in two distinct ways:
   a) Skim through the program by moving quickly between lines.
   b) Read the content of lines with features using the left/right arrow keys to navigate character by character (or word by word).
4. Adjust the accessibility.signals.lineFeatureDelays.critical and accessibility.signals.lineFeatureDelays.informational settings to modify the delays. Observe how these changes influence the user experience.

Future work

The new methodology renders the accessibility.signals.debouncePositionChanges setting obsolete. Consequently, the debouncing code previously used in accessibilitySignalLineFeatureContribution.ts has been eliminated. This code was not only redundant but could also inadvertently prolong the new delays beyond their intended duration. While this setting is currently ineffective, it is being retained temporarily. It may serve as a universal switch for all future delay adjustments. If this approach is adopted, a renaming of the setting would be appropriate to reflect its broader role.

[ramoncorominas]

    @microsoft-github-policy-service agree

[ramoncorominas]

@microsoft-github-policy-service agree

[ramoncorominas]

cc: @rperez030

I have made two additional commits since the original pull request. These updates introduce user-configurable settings for the delays and remove the previously used debouncing code, which has become redundant and indeed could interfere with the new delays, extending them beyond their intended duration. With these updates, I consider that the new feature is now complete.

I have decided to retain the accessibility.signals.debouncePositionChanges setting for the time being. Although it is not utilized in the current version, it could potentially serve as a global toggle for all new delays in future updates. If this approach is adopted, a renaming of the setting would be appropriate to reflect its broader role.

Please review these changes at your earliest convenience. I look forward to your feedback and suggestions.

[hediet]

Thanks for the PR! Unfortunately, I'm not able to understand the PR description.

This foundational change allows for the separation and individual handling of different modalities based on various conditions like triggers or settings.

What do you mean with that?

These changes collectively provide a more responsive and user-centric accessibility signals feature in VS Code, enhancing the overall user experience.

This statement needs some convincing arguments...

---

The code change might be good and justified, but I just don't understand the reasoning behind it.

[hediet]

Please update the description to reflect what this PR is about.
What problem does SignalModality solve?

[ramoncorominas]

@hediet, Thanks for taking the time to review this PR, I've updated the description with a detailed explanation of the issues in accessibility signals that this PR intends to solve. I'm unsure if I need to follow a specific template for the description. I noticed a new issue that might suggest this, but navigating the pull request page with my screen reader this point isn't clear to me.

[ramoncorominas]

PR description updated

[hediet]

Will look into this next week, this week we have testing!

[meganrogge]

Layering issue:

[valid-layers-check      ] [build/lib/layersChecker.ts]: Reference to symbol 'NodeJS' from '@types/node' violates layer '**/vs/**/browser/**' (/mnt/vss/_work/1/s/src/vs/workbench/contrib/accessibilitySignals/browser/accessibilitySignalLineFeatureContribution.ts (46,59) Learn more about our source code organization at https://github.com/microsoft/vscode/wiki/Source-Code-Organization.

[meganrogge]

Thanks for working on this. It works well based on my testing 👏🏼

[ramoncorominas]

Layering issue:

[valid-layers-check      ] [build/lib/layersChecker.ts]: Reference to symbol 'NodeJS' from '@types/node' violates layer '**/vs/**/browser/**' (/mnt/vss/_work/1/s/src/vs/workbench/contrib/accessibilitySignals/browser/accessibilitySignalLineFeatureContribution.ts (46,59) Learn more about our source code organization at https://github.com/microsoft/vscode/wiki/Source-Code-Organization.

I have changed the NodeJS.timeout type to any, I tried to change it to number but the linter complains... Maybe there is a better option in the context of the VSCode repo, but I've not found it, I did a quick search in the repo for other setTimeout uses but I could not see a type definition.

[ramoncorominas]

My apologies, I closed the PR by mistake 😳

[ramoncorominas]

Thanks for working on this. It works well based on my testing 👏🏼

Thanks for your feedback and for taking the time to test my work, I really appreciate it 👏🏼. If there are any specific areas you think could be improved or require further clarification, please let me know. I look forward to any additional comments you may have ☺

[rperez030]

@ramoncorominas I cannot thank you enough for all the work and time spent on this. You definitely took my ramblings about delays to the next level. I really like the modalities approach. @meganrogge this work well on Windows as far as I can tell. I don't have my environment properly setup to test on macOS, so I may wait until it can be merged to test over there.

[meganrogge]

Thanks for testing on Windows @rperez030. I tested it on macOS 👍🏼

[hediet]

I had a more detailed review now.

Over the years, the SignalLineFeatureContribution changed in many (unintended) ways and is not in a good shape anymore.
Ideally, a LineFeature is stateless, but _lineChanged works against that.

Before changing the code more, I'd suggest to describe this entire feature in a detailed way first (please without chat gpt).
Also, there are two things that can be debounced: Cursor changes and feature (meaning markers, breakpoint etc.) changes.
For example, a warning might appear for some milliseconds and then disappear again.

I also don't fully understand the Voice modality. How is it different from an aria alert?

[rperez030]

I also don't fully understand the Voice modality. How is it different from an aria alert?

An aria alert role tells a screen reader that changes in a particular container are relevant so that they can be announced via Braille or speech, what we today call announcements. I understand the voice modality to be specific about TTS. For example, someone could build an extension to support magnification users who are not screen reader users that makes voice announcements in certain situations depending on these settings. Someone who uses a screen reader may benefit from hearing certain events announced by a different voice in a process that is separate from the screen reader. Someone may build an extension that listens for haptic signals to transmit haptic feedback to a user via a wristband.

Also, @hediet, I'm not saying this is the case, but some folks who have English as their second language, or who do not speak English at all but want to contribute may use Copilot or any translation tool to help with their writing, or actually use the Copilot feature to generate commit messages, for example. I suggest that we focus on the actual content, the accuracy of the explanation, the potential benefit of the contribution, and the quality of the code rather than the tools contributors use to get things done.

[hediet]

Someone who uses a screen reader may benefit from hearing certain events announced by a different voice in a process that is separate from the screen reader.

That sounds interesting. How would that work technically? Currently, signals are not forwarded to extensions, so support for this would have to be implemented in VS Code core.
I'm just concerned to add an enum variant (Voice) for something that is not implemented yet (or even worse, not implementable), as it is just dead code.
So far it looks like the Modality enum just reflects the existing audio-cue/aria-alert settings and enables feature-category/modal-specific delays.

A document that describes how these range/line-based "features" should work would be very helpful.

Also, @hediet, I'm not saying this is the case, but some folks who have English as their second language, or who do not speak English at all but want to contribute may use Copilot or any translation tool to help with their writing, or actually use the Copilot feature to generate commit messages, for example.

I agree and respect that, but the text below "Enhanced Solution Strategy" in the initial PR description is extremely difficult to read and to make sense of - and this is very typical for chat gpt generated text. The text above it however is very clear and precise (thanks for adding it!).

[hediet]

Superseded by #210679. Given changed requirements of the feature and new insights, we decided to rewrite the line-feature based signals (now called text property signals). This cleans up the code significantly and gives a lot of flexibility with regards to delay times.

Thanks for the idea of per-modality-delays!