Fix Voice Over accessibility for title in TableViewHeaderFooterView w… #1197
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
…hen presented in popover ISSUE - When Voice Over accessibility is enabled, the TableViewHeaderFooterView title has a wrong selection when presented in a popover on iPad. - The title in this header/footer is a UITextView and in the existing implementation we override the default accessibility behavior by making the UITextView an accessibility element and inserting the accessibility trait as header for the case when TableViewHeaderFooterView is used as a header. This implementation leads to a wrong text selection for title in popover on iPad. FIX - A quick fix is to remove the accessibility element assignment from UITextView and keep the accessibility trait insertion as header but the issue in this case is that voice over selection is still buggy for the case when the title has multiple lines of text for both cases when the TableViewHeaderFooterView is used as a header or as a footer. - A fix that works properly for both header and footer for one line of multiple lines of text is to remove the custom logic added for accessibility (.heading accessibility traits) and keep the system default implementation. For UITextView the default implementation behaves like .staticText accessibility trait so in this case the voice over will present just the header title text not followed by ‘heading’ specification. - A fix that would work with .heading specification is to use a UILabel instead of UITextView for the title, but that would imply custom implementation for hyper-texts. That would impact the TableViewHeaderFooterView interface signature because we would have to mention the clickable portion for the label text in order to specify it in the attributed string range and then would be assigned to the label. (Maybe that can be done later.) PRODUCT IMPACT - This change impact Voice Over accessibility in table view headers and footers in W/X/P/O/OM for iPad and iPhone. VERIFICATION - Verified the change for Voice Over accessibility in table header and footer in popovers or in presented controllers. - Verified that the Voice Over selection works well for header and footer when there is one line title and multiple lines title only; multiple lines and single line title with accessory view. - Verified that Voice Over accessibility works well in portrait/landscape and split view modes for iPad and iPhone.
harrieshin
reviewed
Aug 29, 2022
|
Fix looks good to me, but probably should make sure @harrieshin is explicitly good with it and probably update the PR notes to make clear which of the proposed fixes we went with |
|
would you please update your description with video include with the voiceover captions specially around the footer with link to make sure it isn't regressed? |
|
Might also want to update the table markdown in your description so that the side-to-side comparison is easier to look at |
…applicable anymore.VO accessibility works well for hyper-text in UITextView when .header trait is removed.
harrieshin
approved these changes
Aug 30, 2022
11 tasks
11 tasks
edjamesmsft
added a commit
that referenced
this pull request
Sep 27, 2022
* make sure macOS Avatar respect custom color (#1196) -new private var to bookkeep when the colors are meant to be custom vs default -remove unnecessary initialsTextField background coloring. It should be just handled by the initailsView itself. -add a test case in TestAvatarController to have avatar hosted by NSPopover * Fix Voice Over accessibility for title in TableViewHeaderFooterView w… (#1197) ISSUE When Voice Over accessibility is enabled, the TableViewHeaderFooterView title has a wrong selection when presented in a popover on iPad. The title in this header/footer is a UITextView and in the existing implementation we override the default accessibility behavior by making the UITextView an accessibility element. This implementation leads to a wrong text selection for title in popover on iPad. FIX The fix is to remove the accessibility element assignment from UITextView and keep the accessibility trait insertion as .header when TableViewHeaderFooterView is used as header and remove the accessibility trait insertion when the view is used as footer. This implementation already exists but it was followed by a 'Bug in iOS - ...' implementation where accessibility element for UITextView was set up to 'false' and then back to 'true'. That implementation broke the VO selection for multi-line of text title for both header and footer scenarios. Removing that logic fixed the issue properly. * introduced a Medium Fluent Button Size * Fixed swiftlint issues * Add Objective C demo for PopupMenu (#1198) * Add Objective C demo for PopupMenu * Move setup to loadVIew * Set stack alignment to top to resize button * Remove comments * Add selected image for Toronto so it isn't a blue square * Fix indentation * Fix whitespace * Move ObjC to middle of name * Merging `ControlTokenSet` to main branch (#1206) * Merging ControlTokenSet from fluent2-tokens * Delete accidental file merge * Restore lost code in separator merge (#1212) * Revert "PillButtonBar: Fixing warnings for iOS 15 target deployment version. (#1165)" (#1213) This reverts commit 8d26fe7. * Align Notification Border with Figma (#1203) * Align Notification with Figma * Revert naming and token change * Align Notification with Figma * Revert naming and token change * Rebase onto main * Delete dupe edge border * Revert removing default bool * Implement functionality to show a custom view within a CommandBarItem (#1210) Implement functionality to show a custom view within a CommandBarItem * cherry-pick 45c022d (#1217) Co-authored-by: Sophia Lee <sophia.lee0416@gmail.com> * sanitization * sanitization * Fix tokens, text width, and showDefaultDismissActionButton demo (#1215) * Remove all usage of UIScreen.main (#1216) UIScreen.main is deprecated in iOS 16 (https://developer.apple.com/documentation/uikit/uiscreen/1617815-main). Most of the usage of UIScreen main is legacy code that tries to figure out device pixel. But, in reality, in order to avoid anti-aliasing we should be just making sure we draw on integer points. If we are trying to draw border, let's just draw on half point for now. * update demo windowscene * demo app only: tableviewcell accessory adhering to 1pt borderwidth and 2pt corner radius which are closer to fluent 2 design tokens * remove usage of UIScreen.middleOrigin * remove usage of roundDownToDevicePixels * remove usuage of deviceLineHeight. just use UIFont's lineheight * delete UIFont extension. * remove usage of roundToDevicePixels * remove UIScreen+extension * Separator height to be half a point * remove UITableViewCell+extension * remove uiscreen.main.bounds usage * remove file reference that doesn't exist * use guard let to figure out the window for drawercontroller * Update GradientInfo to specify the gradient is linear (#1218) * Update documentation to be explicit about linear * Add Linear to name of GradientInfo * Remove UIEdgeInsets from SegmentedPillButton (#1221) * Remove UIEdgeInsets from SegmentedPillButton * Add TODO * Move `fluentTheme` from window to view (#1219) * Move `fluentTheme` from window to view * Add logic to only refresh if view is a descendent of the newly themed view * allow colorful image in notification toast Currently we always render the toast image as template. If the client team pass in image that is set "renderAsOriginal" respect that mode. * Added a minButtonHeight to account for the fact that the button height currently grows to fit the image size within it * add new module map in the fluentuilib and add copy phase (#1231) use FluentUI as a clang module in objc, we want to publish module.modulemap next to the fluentui-swift.h * Removing spacing and padding values from token sets (#1220) * First set of controls without spacing tokens * Complete the rest of the controls * Restoring in-use values * Move layout constants to TokenSets * Revert Notification Colors to Fluent 1 Tokens (#1226) * CommandBar: Update group spacing and add large content viewer support (#1229) * Decrease CommandBar item group spacing to 8. Change received approval from the Fluent Design team. * Enable LargeContentViewer for the CommandBar * Instantiating state variables in HUD (#1233) * added min button heights for small and large sizes * Update AttributedText to scale (#1234) * no longer need .ado folder (#1232) * Bumping FluentUI version (0.8.0) (#1230) * sanitization. Addressed PR Comments * Fix RTL behavior for CommandBar Pinned Items (#1236) It was discovered that the RTL behavior for the pinned items in CommandBar was off. The leading and trailing groups were switching sides as expected, but the additional transform being applied `configureHierarchy` is causing the views inside the group to be shown in LTR order. Fix: Remove the transform for the leading and trailing groups inside `configureHierarchy` and let the default behavior from the UIStackView handle the transition. * Update Avatar size ramp (#1194) * Renamed avatar sizes * Added avatar size 20 and size 56 * Added new sizes in demo controllers * Removed unused variable in AvatarGroupDemoController * Moved changes to token sets * Moved size tokens outside of AvatarTokenSet to fix conflicts * Bottom sheet collapsed height resolver API (#1207) * Revert "Revert "PillButtonBar: Fixing warnings for iOS 15 target deployment version. (#1165)" (#1213)" (#1244) This reverts commit 518bdc6. * Add swipe to hide API to bottom sheet (#1242) * Resolved merge and build errors * Removing unnecessary AvatarTokens.swift * Trailing whitespace Co-authored-by: Harrie Shin <hyshin@microsoft.com> Co-authored-by: isatoderici <65185855+isatoderici@users.noreply.github.com> Co-authored-by: Amogh Vinod Aralimatti <amaralim@microsoft.com> Co-authored-by: Mike Schreiber <mischreiber@microsoft.com> Co-authored-by: huwilkes <67026548+huwilkes@users.noreply.github.com> Co-authored-by: Jeanie Huynh <31874971+jeaniehuynh@users.noreply.github.com> Co-authored-by: Des Marks <johnmarks@microsoft.com> Co-authored-by: Sophia Lee <sophia.lee0416@gmail.com> Co-authored-by: Amogh Vinod Aralimatti <moggogogo@gmail.com> Co-authored-by: Lamine Male <106181067+laminesm@users.noreply.github.com> Co-authored-by: Lukas Capkovic <3610850+lcapkovic@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…hen presented in popover
Platforms Impacted
Description of changes
ISSUE
When Voice Over accessibility is enabled, the TableViewHeaderFooterView title
has a wrong selection when presented in a popover on iPad.
The title in this header/footer is a UITextView and in the existing implementation we override the default accessibility behavior by making the UITextView an accessibility element.
This implementation leads to a wrong text selection for title in popover on iPad.
FIX
A NICE TO HAVE
Verification
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow