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.

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

Support for text review commands on object name in Visual Studio Code #16248

Merged
merged 11 commits into from
Jun 26, 2024

Conversation

cary-rowen
Copy link
Contributor

@cary-rowen cary-rowen commented Mar 2, 2024

Link to issue number:

Fixed #16246

Summary of the issue:

Most controls in Visual Studio Code do not support text review commands.

The most typical example is when the user wants to review the control name character by character, such as the file name in the file tree and the setting items on the settings page. Based on current experience, this is not easy to achieve.

Description of user facing changes

Users will be able to review control name in Visual Studio Code by NVDA’s text review commands.

Description of development approach

In the application module for Visual Studio Code, a modification has been made within the event_NVDAObject_init method to enhance the handling of objects.
Specifically, for objects whose role is not EDITABLETEXT , the TextInfo property is now explicitly set to NVDAObjectTextInfo.

Testing strategy:

Manual testing

Known issues with pull request:

unknown

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

Summary by CodeRabbit

  • New Features
    • Introduced support for text review commands for an object's name in Visual Studio Code to enhance accessibility.
  • Bug Fixes
    • Resolved issues related to NVDA interactions with Windows 11.

@cary-rowen cary-rowen requested a review from a team as a code owner March 2, 2024 01:28
@cary-rowen cary-rowen requested a review from gerald-hartig March 2, 2024 01:28
@cary-rowen cary-rowen changed the title Vs code text info Support for text review commands on object name in Visual Studio Code Mar 2, 2024
@CyrilleB79
Copy link
Collaborator

Is the issue being fixed here only applicable to VSCode?
Or could this fix be included in more general NVDA objects?

source/appModules/code.py Outdated Show resolved Hide resolved
source/appModules/code.py Outdated Show resolved Hide resolved
user_docs/en/changes.t2t Outdated Show resolved Hide resolved
@seanbudd seanbudd marked this pull request as draft March 5, 2024 00:13
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 5, 2024
@CyrilleB79
Copy link
Collaborator

The issue being fixed here seems the same as the one in #14088.
In case the cause is the same, could this fix be generalized to more object types or duplicated for Chrome?

@cary-rowen
Copy link
Contributor Author

Hi @CyrilleB79

The issue being fixed here seems the same as the one in #14088 .
In case the cause is the same, could this fix be generalized to more object types or duplicated for Chrome?

I haven't investigated #14088 yet, could it be a duplicate of #15159?

@cary-rowen cary-rowen requested a review from seanbudd March 5, 2024 08:18
@CyrilleB79
Copy link
Collaborator

I haven't investigated #14088 yet, could it be a duplicate of #15159?

@cary-rowen, #14088 is about the menus in Chrome application itself, whereas #15159 is about web content displayed by the Chrome browser.
I cannot tell you if these two issues are due to the same root cause, but there is nothing obvious that would let it think.

Could you please have a deeper look at #14088 to see if it may be related to the current issue?

@cary-rowen
Copy link
Contributor Author

cary-rowen commented Mar 5, 2024

#14088 can indeed be fixed using the same idea as this PR.

But I'd like to hear some best practices on how to implement a more general fix.

@CyrilleB79
Copy link
Collaborator

#14088 can indeed be fixed using the same idea as this PR.

But I'd like to hear some best practices on how to implement a more general fix.

@seanbudd can you help here?

@seanbudd
Copy link
Member

@coderabbitai can you suggest a more general fix using the idea from this PR to also fix #14088

Otherwise, I think this PR can be accepted as-is

@seanbudd seanbudd marked this pull request as ready for review June 17, 2024 00:05
Copy link
Contributor

coderabbitai bot commented Jun 17, 2024

Tip

For best results, initiate chat on the files or code changes.

@seanbudd, after reviewing the details of issue #14088 and the modifications made in this PR, it appears that a generalized fix could indeed be applied to address similar issues in different applications, including Chrome.

Analysis and Suggested General Fix

The root cause in both issues (Visual Studio Code and Chrome menus) involves handling non-editable text elements where NVDA fails to provide detailed character-by-character or word-by-word navigation. The PR for Visual Studio Code addresses this by setting the TextInfo property to NVDAObjectTextInfo for non-editable text objects.

For a more general fix applicable to both Visual Studio Code and Chrome, and potentially other applications, we could consider the following approach:

  1. Modify the Global Event Handler: Instead of modifying event_NVDAObject_init specifically in the application module for Visual Studio Code, this method can be adjusted in a more central location that handles all similar events across different applications.
  2. Check for Non-Editable Text Elements: The modified handler should check if an object is a non-editable text element.
  3. Set Appropriate TextInfo Property: If the object is non-editable, set the TextInfo property to a new or existing TextInfo class that is designed to enhance text review commands. This class would need to handle the peculiarities of different applications, possibly by recognizing specific UI frameworks or element properties.
  4. Optimization and Performance: Ensure that the implementation is optimized to avoid performance degradation, especially in applications with a large number of UI elements.

Implementation Example

Here’s a pseudocode example of how this might look:

def global_event_NVDAObject_init(obj):
    if obj.role == ROLE_TEXT and not obj.state.editable:
        # This is a simplification; actual implementation may need to be more sophisticated
        obj.TextInfo = EnhancedNVDAObjectTextInfo if is_applicable(obj) else DefaultTextInfo

def is_applicable(obj):
    # Determine if the object belongs to an application or a UI framework that requires enhanced handling
    return obj.appModule.appName in ['chrome', 'code', 'other_app']

This approach centralizes the handling of non-editable text elements, making it easier to manage and extend to other applications as needed.

Let me know if this aligns with your expectations for a general fix, or if further adjustments are needed!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Fetching the description and any relevant code snippets or comments for issue #14088 from GitHub
gh issue view 14088 --repo nvaccess/nvda --json body,comments
# Fetching the implementation details of the modifications made in this PR
gh pr view 16248 --repo nvaccess/nvda --json files

Length of output: 14280

@seanbudd
Copy link
Member

@coderabbitai full review

Copy link
Contributor

coderabbitai bot commented Jun 18, 2024

Walkthrough

The changes enhance visual accessibility support in NVDA for Visual Studio Code by introducing text review commands for inspecting object names character by character. Key modifications include importing NVDAObjectTextInfo, adding an initialization event, and altering the method for choosing overlay classes.

Changes

File Path Change Summary
source/appModules/code.py Import NVDAObjectTextInfo, add event_NVDAObject_init, modify chooseNVDAObjectOverlayClasses
user_docs/en/changes.t2t Document support for text review commands and Windows 11 interaction fixes

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant NVDA
    participant VSCode

    User->>VSCode: Navigate file tree
    VSCode->>NVDA: Send an NVDAObject
    NVDA->>NVDA: event_NVDAObject_init
    NVDA->>VSCode: Request text info
    VSCode-->>NVDA: Provide text info
    User->>NVDA: Use text review commands
    NVDA-->>User: Present text character by character
Loading

Assessment against linked issues

Objective Addressed Explanation
Support for text review commands in Visual Studio Code (#16246)

The modifications meet the specified objectives by facilitating character-wise text review for elements within Visual Studio Code, thus improving accessibility.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range and nitpick comments (2)
source/appModules/code.py (1)

28-35: Consider a more general solution for non-editable text elements.

Based on the discussion in the PR comments, there's interest in a more generalized solution for handling non-editable text elements across different applications. This specific implementation for Visual Studio Code could be adapted into a more flexible, application-agnostic approach. Consider exploring the use of a decorator or a higher-order function that can be applied universally across different app modules.

user_docs/en/changes.t2t (1)

71-71: Ensure consistency in documentation by maintaining a uniform format for all entries.

Consider using a consistent format for all change log entries, including the mapping of keys and the description of bug fixes and feature enhancements.

source/appModules/code.py Show resolved Hide resolved
@seanbudd
Copy link
Member

@cary-rowen any thoughts on the general approach suggestions?

@cary-rowen
Copy link
Contributor Author

I haven't tried it yet, based on the robot's suggestions:

Modify the Global Event Handler : Instead of modifying event_NVDAObject_init specifically in the application module for Visual Studio Code, this method can be adjusted in a more central location that handles all similar events across different applications.

Sounds reasonable, but I'm not sure if it's possible, I would really prefer to apply a more general approach to fix #14088

@seanbudd seanbudd merged commit 3c981e2 into nvaccess:master Jun 26, 2024
1 check passed
seanbudd pushed a commit that referenced this pull request Jan 9, 2025
…ble controls in Visual Studio Code (#17573)

closed #17525

Summary of the issue:
In Visual Studio Code, NVDA was unable to correctly read certain non-standard editable text controls, specifically those with the SECTION role that have an editable state. These controls could be edited by users but were not recognized as EDITABLETEXT by NVDA.

Description of user-facing changes:
This fix ensures that NVDA correctly recognizes and reads non-standard editable text controls, such as those with the SECTION role and an editable state, in Visual Studio Code.

Description of development approach:
In #16248, we introduced support for text-review commands for objects in Visual Studio Code. Initially, we used obj.role == controlTypes.Role.EDITABLETEXT as the check for editable text fields. However, this caused a regression for some controls, specifically SECTION elements in VS Code that were editable but did not have the EDITABLETEXT role. These controls have an editable state but were overlooked by NVDA. This fix updates the logic to account for such cases, ensuring these elements are treated as editable controls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NVDA does not support text review commands in controls in Visual Studio Code.
3 participants