-
-
Notifications
You must be signed in to change notification settings - Fork 654
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
soffice: Report keyboard-triggered font size change #17147
Merged
seanbudd
merged 3 commits into
nvaccess:master
from
michaelweghorn:michaelweghorn/announce_font_size_change
Sep 10, 2024
Merged
soffice: Report keyboard-triggered font size change #17147
seanbudd
merged 3 commits into
nvaccess:master
from
michaelweghorn:michaelweghorn/announce_font_size_change
Sep 10, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Link to issue number: Partially implements feature requests from nvaccess#6915 ### Summary of the issue: commit 46a3436 ("soffice: Report keyboard-triggered formatting toggles in Writer (nvaccess#16413)") implemented announcement of formatting changes triggered by keyboard shortcuts for formatting attributes whose state is represented by toggle buttons in Writer's formatting toolbar. Issue nvaccess#6915 requests announcement of further text formatting attributes that are not represented by simple toggle buttons. This includes decreasing/increasing the font size, which can be done using the Ctrl+[ and Ctrl+] shortcuts when using an English (US) UI and keyboard layout. ### Description of user facing changes When decreasing or increasing the font size in LibreOffice Writer using the corresponding keyboard shortcuts, NVDA announces the new font size. ### Description of development approach Extend the solution from the above-mentioned commit 46a3436 to not only cover toggle buttons, but also UI controls implementing the IAccessibleText interface and handle the gestures/keyboard shortcuts for changing the font size: * Rename methods and variables introduced earlier to not have button-specific names. * Extract some logic to helper methods/classes to avoid duplication in `SymphonyButton` and the newly introduced `SymphonyText` logic. * Add Ctrl+[ and Ctrl+] to the list of keyboard gestures to handle. * Add `SymphonyText.event_valueChange` override that announces the new value. This gets triggered when the value of the "Font Size" editable combobox in Writer's formatting toolbar changes after using the keyboard shortcut, and results in the new value being announced, e.g. "14 pt". This is comparable to the handling in `SymphonyButton.event_stateChange` introduced in the earlier above-mentioned commit. * Increase the timeout for announcement of events from 0.15 to 2 seconds, as 0.15 seconds wasn't always sufficient when testing the new feature. ### Testing strategy: 1. start NVDA 2. start LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+[ to decrease the font size 4. Verify that NVDA announces the new font size, e.g. "10 pt" when using the default document template 5. Press Ctrl+] to increase the font size 4. Verify that NVDA announces the new font size, e.g. "12 pt" when using the default document template 5. repeat the tests mentioned in commit 46a3436 to verify the toggle button case still works as expected ### Known issues with pull request: None ### Code Review Checklist: - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken.
seanbudd
added
the
conceptApproved
Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
label
Sep 10, 2024
seanbudd
reviewed
Sep 10, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @michaelweghorn - looks almost ready to go
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
While issue nvaccess#16413 also requested announcement of formatting changes, PR nvaccess#17147 is for issue nvaccess#6915.
seanbudd
approved these changes
Sep 10, 2024
michaelweghorn
added a commit
to michaelweghorn/nvda
that referenced
this pull request
Sep 10, 2024
### Link to issue number: Partially implements feature requests from nvaccess#6915 ### Summary of the issue: Commit 20bdb39 ("soffice: Report keyboard-triggered font size change (nvaccess#17147)") implemented announcement of the new value of the font size combobox in Writer's formatting toolbar when it is changed via keyboard shortcut. Issue nvaccess#6915 requests announcement of further attributes, including the announcement of the current paragraph style when applying "Heading 1",..., "Heading 5" styles via the Ctrl+1,..., Ctrl+5 keyboard shortcuts. ### Description of user facing changes When applying the "Body Text" or a heading paragraph style using the corresponding keyobard shortcut in LibreOffice Writer 25.2 or newer, NVDA announces the new paragraph style. ### Description of development approach Extend the solution from the above-mentioned commit 20bdb39. Other than the simpler case of the font size combo box, changing the paragraph style can imply more related formatting changes (e.g. font size, bold,...). To avoid triggering the announcement of these implicit formatting changes, but rather announce the new paragraph style to confirm that the user-triggered action had the expected result, introduce a way to reliably identify the paragraph style combobox in the formatting toolbar: 1) Add an "id" object attribute to the IAccessible2 object attribute specification: > This identifier remains the same across different sessions of > the same application, regardless of the UI language. IAccessible2 commit: LinuxA11y/IAccessible2@2b8c2c7 2) In LibreOffice, set the "id" attribute to the ID already used in the UI file format (GtkBuilder XML files): https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53 3) In NVDA, extend the logic for announcing gesture-triggered formatting changes to allow specifying a specific ID of the UI element(s) of interest. If one is set, don't announce changes in other UI elements for that gesture. Set the ID to the one that the paragraph style combobox in Writer's formatting toolbar has ("applystyle"). Let NVDA handle the Ctrl+0, Ctrl+1, Ctrl+2, Ctrl+3, Ctrl+4 and Ctrl+5 gestures to announce the new paragraph style ("Body Text" for Ctrl+0, corresponding heading for Ctrl+1 through Ctrl+5). ### Testing strategy: 1. start NVDA 2. start current development version of LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+1 to apply paragraph Style "Heading 1" 4. Verify that NVDA announces the new paragraph style by name, i.e. says "Heading 1" 5. Repeat steps 3 and 4 with Ctrl+2, Ctrl+3, Ctrl+4, Ctrl+5 to verify that the other heading levels are also announced correctly. 6. Press Ctrl+0 to apply paragraph style "Body Text" and verify that NVDA announces the new paragraph style by name, i.e. says "Body Text" 8. Retest that the other supported keyboard gestures as implemented in commits 46a3436 and 20bdb39 are still announced as expected. ### Known issues with pull request: None, but note that a current LibreOffice development version (to become LibreOffice 25.2) is required for the feature to actually work. ### Code Review Checklist: - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken.
michaelweghorn
added a commit
to michaelweghorn/nvda
that referenced
this pull request
Sep 10, 2024
### Link to issue number: Partially implements feature requests from nvaccess#6915 ### Summary of the issue: Commit 20bdb39 ("soffice: Report keyboard-triggered font size change (nvaccess#17147)") implemented announcement of the new value of the font size combobox in Writer's formatting toolbar when it is changed via keyboard shortcut. Issue nvaccess#6915 requests announcement of further attributes, including the announcement of the current paragraph style when applying "Heading 1",..., "Heading 5" styles via the Ctrl+1,..., Ctrl+5 keyboard shortcuts. ### Description of user facing changes When applying the "Body Text" or a heading paragraph style using the corresponding keybard shortcut in LibreOffice Writer 25.2 or newer, NVDA announces the new paragraph style. ### Description of development approach Extend the solution from the above-mentioned commit 20bdb39. Other than the simpler case of the font size combo box, changing the paragraph style can imply more related formatting changes (e.g. font size, bold,...). To avoid triggering the announcement of these implicit formatting changes, but rather announce the new paragraph style to confirm that the user-triggered action had the expected result, introduce a way to reliably identify the paragraph style combobox in the formatting toolbar: 1) Add an "id" object attribute to the IAccessible2 object attribute specification: > This identifier remains the same across different sessions of > the same application, regardless of the UI language. IAccessible2 commit: LinuxA11y/IAccessible2@2b8c2c7 2) In LibreOffice, set the "id" attribute to the ID already used in the UI file format (GtkBuilder XML files): https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53 3) In NVDA, extend the logic for announcing gesture-triggered formatting changes to allow specifying a specific ID of the UI element(s) of interest. If one is set, don't announce changes in other UI elements for that gesture. Set the ID to the one that the paragraph style combobox in Writer's formatting toolbar has ("applystyle"). Let NVDA handle the Ctrl+0, Ctrl+1, Ctrl+2, Ctrl+3, Ctrl+4 and Ctrl+5 gestures to announce the new paragraph style ("Body Text" for Ctrl+0, corresponding heading for Ctrl+1 through Ctrl+5). ### Testing strategy: 1. start NVDA 2. start current development version of LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+1 to apply paragraph Style "Heading 1" 4. Verify that NVDA announces the new paragraph style by name, i.e. says "Heading 1" 5. Repeat steps 3 and 4 with Ctrl+2, Ctrl+3, Ctrl+4, Ctrl+5 to verify that the other heading levels are also announced correctly. 6. Press Ctrl+0 to apply paragraph style "Body Text" and verify that NVDA announces the new paragraph style by name, i.e. says "Body Text" 8. Retest that the other supported keyboard gestures as implemented in commits 46a3436 and 20bdb39 are still announced as expected. ### Known issues with pull request: None, but note that a current LibreOffice development version (to become LibreOffice 25.2) is required for the feature to actually work. ### Code Review Checklist: - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken.
5 tasks
michaelweghorn
added a commit
to michaelweghorn/nvda
that referenced
this pull request
Sep 10, 2024
### Link to issue number: Partially implements feature requests from nvaccess#6915 ### Summary of the issue: Commit 20bdb39 ("soffice: Report keyboard-triggered font size change (nvaccess#17147)") implemented announcement of the new value of the font size combobox in Writer's formatting toolbar when it is changed via keyboard shortcut. Issue nvaccess#6915 requests announcement of further attributes, including the announcement of the current paragraph style when applying "Heading 1",..., "Heading 5" styles via the Ctrl+1,..., Ctrl+5 keyboard shortcuts. ### Description of user facing changes When applying the "Body Text" or a heading paragraph style using the corresponding keyboard shortcut in LibreOffice Writer 25.2 or newer, NVDA announces the new paragraph style. ### Description of development approach Extend the solution from the above-mentioned commit 20bdb39. Other than the simpler case of the font size combo box, changing the paragraph style can imply more related formatting changes (e.g. font size, bold,...). To avoid triggering the announcement of these implicit formatting changes, but rather announce the new paragraph style to confirm that the user-triggered action had the expected result, introduce a way to reliably identify the paragraph style combobox in the formatting toolbar: 1) Add an "id" object attribute to the IAccessible2 object attribute specification: > This identifier remains the same across different sessions of > the same application, regardless of the UI language. IAccessible2 commit: LinuxA11y/IAccessible2@2b8c2c7 2) In LibreOffice, set the "id" attribute to the ID already used in the UI file format (GtkBuilder XML files): https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53 3) In NVDA, extend the logic for announcing gesture-triggered formatting changes to allow specifying a specific ID of the UI element(s) of interest. If one is set, don't announce changes in other UI elements for that gesture. Set the ID to the one that the paragraph style combobox in Writer's formatting toolbar has ("applystyle"). Let NVDA handle the Ctrl+0, Ctrl+1, Ctrl+2, Ctrl+3, Ctrl+4 and Ctrl+5 gestures to announce the new paragraph style ("Body Text" for Ctrl+0, corresponding heading for Ctrl+1 through Ctrl+5). ### Testing strategy: 1. start NVDA 2. start current development version of LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+1 to apply paragraph Style "Heading 1" 4. Verify that NVDA announces the new paragraph style by name, i.e. says "Heading 1" 5. Repeat steps 3 and 4 with Ctrl+2, Ctrl+3, Ctrl+4, Ctrl+5 to verify that the other heading levels are also announced correctly. 6. Press Ctrl+0 to apply paragraph style "Body Text" and verify that NVDA announces the new paragraph style by name, i.e. says "Body Text" 8. Retest that the other supported keyboard gestures as implemented in commits 46a3436 and 20bdb39 are still announced as expected. ### Known issues with pull request: None, but note that a current LibreOffice development version (to become LibreOffice 25.2) is required for the feature to actually work. ### Code Review Checklist: - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken.
seanbudd
pushed a commit
that referenced
this pull request
Sep 17, 2024
Partially implements feature requests from #6915 Summary of the issue: Commit 20bdb39 ("soffice: Report keyboard-triggered font size change (#17147)") implemented announcement of the new value of the font size combobox in Writer's formatting toolbar when it is changed via keyboard shortcut. Issue #6915 requests announcement of further attributes, including the announcement of the current paragraph style when applying "Heading 1",..., "Heading 5" styles via the Ctrl+1,..., Ctrl+5 keyboard shortcuts. Description of user facing changes When applying the "Body Text" or a heading paragraph style using the corresponding keyboard shortcut in LibreOffice Writer 25.2 or newer, NVDA announces the new paragraph style. Description of development approach Extend the solution from the above-mentioned commit 20bdb39. Other than the simpler case of the font size combo box, changing the paragraph style can imply more related formatting changes (e.g. font size, bold,...). To avoid triggering the announcement of these implicit formatting changes, but rather announce the new paragraph style to confirm that the user-triggered action had the expected result, introduce a way to reliably identify the paragraph style combobox in the formatting toolbar: Add an "id" object attribute to the IAccessible2 object attribute specification: This identifier remains the same across different sessions of the same application, regardless of the UI language. IAccessible2 commit: LinuxA11y/IAccessible2@2b8c2c7 In LibreOffice, set the "id" attribute to the ID already used in the UI file format (GtkBuilder XML files): https://git.libreoffice.org/core/commit/21b29f25660db973fa1480de77e6a69d76a5de53 In NVDA, extend the logic for announcing gesture-triggered formatting changes to allow specifying a specific ID of the UI element(s) of interest. If one is set, don't announce changes in other UI elements for that gesture. Set the ID to the one that the paragraph style combobox in Writer's formatting toolbar has ("applystyle"). Let NVDA handle the Ctrl+0, Ctrl+1, Ctrl+2, Ctrl+3, Ctrl+4 and Ctrl+5 gestures to announce the new paragraph style ("Body Text" for Ctrl+0, corresponding heading for Ctrl+1 through Ctrl+5).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
api-breaking-change
conceptApproved
Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Partially implements feature requests from #6915
Summary of the issue:
commit 46a3436
("soffice: Report keyboard-triggered formatting toggles in Writer (#16413)") implemented announcement of formatting changes
triggered by keyboard shortcuts for formatting attributes whose state is represented by toggle buttons in Writer's formatting toolbar.
Issue #6915 requests announcement of further text formatting attributes that are not represented by simple toggle buttons. This includes decreasing/increasing the font size, which can be done using the Ctrl+[ and Ctrl+] shortcuts when using an English (US) UI and keyboard layout.
Description of user facing changes
When decreasing or increasing the font size in LibreOffice Writer using the corresponding keyboard shortcuts, NVDA announces the new font size.
Description of development approach
Extend the solution from the above-mentioned
commit 46a3436
to not only cover toggle buttons, but also
UI controls implementing the IAccessibleText
interface and handle the gestures/keyboard shortcuts for changing the font size:
SymphonyButton
and the newly introducedSymphonyText
logic.SymphonyText.event_valueChange
override that announces the new value. This gets triggered when the value of the "Font Size" editable combobox in Writer's formatting toolbar changes after using the keyboard shortcut, and results in the new value being announced, e.g. "14 pt". This is comparable to the handling inSymphonyButton.event_stateChange
introduced in the earlier above-mentioned commit.Testing strategy:
Known issues with pull request:
None
Code Review Checklist:
@coderabbitai summary