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

[fix/a11y] Fix accessibility issues #1349

Merged
merged 31 commits into from
Jun 11, 2024
Merged

[fix/a11y] Fix accessibility issues #1349

merged 31 commits into from
Jun 11, 2024

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Apr 23, 2024

Description

Fixes #1329, #1330, #1333, #1336, #1337, #1338, #1340, #1341.

Related Issue

#1329 : Approved
#1330 : Approved
#1333 : Approved
#1336: Approved
#1337: Approved
#1338: Approved
#1340 : Approved
#1341: Approved

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@felix-schwarz felix-schwarz self-assigned this Apr 23, 2024
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jesmrec
Copy link
Contributor

jesmrec commented May 6, 2024

#1329

Personal space

  • The "Menu" button has an alternative text, but this is in English and appears to be the name of a variable.
    in English and appears to be the name of a variable.
  • The "Plus" button ("+") has an alternative text, but it is in English - "Add item".
    in English - "Add item". As this version of the app is a German version
    a German version, the alternative text should be in German.
  • Note: The list of directories and files contains a "More" button in each
    list item contains a "More" button. To support screen reader users
    users, it would be advisable to display not only the word "More", but also the name of the
    name of the directory or file.

Menu, Shared with me, Spaces

  • The "Menu" button does have an alternative text, but this is in
    English and appears to be the name of a variable.

Shared with others

  • The "Menu" button does have an alternative text, but this is in English
    in English and appears to be the name of a variable.
  • The button with the arrow pointing to the right does have an alternative text
    alternative text, but it is in English and not in German.

Image Viewer

  • The "Menu" button has an alternative text, but this is in English and appears to be the name of a variable.
    in English and appears to be the name of a variable.
  • The alternative text of the "Actions" button contains both the word
    "Actions" as well as the file name, which makes it difficult to understand.
    leads to poor comprehensibility. It is recommended to remove the file name from the alternative text.
    from the alternative text.

NOTE: Translations to german is not an app feature, i assume that existing translations are OK for German language

@jesmrec
Copy link
Contributor

jesmrec commented May 6, 2024

#1330

Personal Space and Shared with me

  • Each directory or file in the list has "status" icons that show users whether the file has already been
    users whether the file has already been shared, for example. Unfortunately
    these icons are not displayed by screen readers, so this information is not accessible to
    not accessible to screen reader users. -> every of this images is spelt together with "Actions available" during dictation. But, those icons are only indicators about the file status and no action is available by clicking on them or any other gesture. is that right @felix-schwarz ?

Image Viewer

  • When focusing on the image with the screen reader, the description of the image is not displayed.
    image, but information such as size and date entries. -> i don't understand what's going on here. What does "description of the image" mean? afaik, we don't describe images, not sure how to check this one

@jesmrec
Copy link
Contributor

jesmrec commented May 7, 2024

#1333

Menu

  • The status of the user is only shown by colored dots. There is no other option is offered to recognize the status, for example through a text alternative.
    • Online (green)
    • Connecting (yellow)
    • Authentication error/Offline (red)

@jesmrec
Copy link
Contributor

jesmrec commented May 7, 2024

#1340

Shared with me

  • The visual labels of the "Accept" and "Reject" buttons are not included in the accessible accessible name, as the accessible name is displayed in English. is output in English.

NOTE: Translations to german is not an app feature, i assume that existing translations are OK for German language

@felix-schwarz felix-schwarz changed the base branch from milestone/12.2 to milestone/12.3 May 7, 2024 08:50
felix-schwarz and others added 12 commits May 13, 2024 10:22
- INIFile+URLFile: simple extension for quickly parsing and composing .url files
- URLDisplayViewController: stub implementation to handle .url files
…ng shortcut URL

- CreateURLShortcutAction: new action to create a URL shortcut in a folder
- design and add "text-uri-list" icon
… open files from file lists, overriding viewers if needed

- OCItem+Interactions: add support for `.directOpen` actions
- INIFile+ShortcutResolution: implement support for shortcuts to other items on the same server
- ShortcutFileDisplayViewController: implement support for shortcuts to other items on the same server, including an item preview
- add new action OpenShortcutFileAction
	- hooks into the `.directOpen` location
	- uses INIFile+ShortcutResolution to open the contained link or jump to the targeted location
	- asks users before opening links
- rename files for better distinctibility:
	- URLDisplayViewController -> ShortcutFileDisplayViewController
	- CreateURLShortcutAction -> CreateShortcutFileAction
- OCItem+UniversalItemListCellContentProvider: fix warning
- CreateShortcutFileAction: break out creation functionality into its own view controller
- CreateShortcutFileViewController:
	- dedicated view controller for creating shortcuts
	- allows picking files and folders for shortcuts to point to and presents an embedded preview for them
	- improved error handling
- INIFile: fix formatting bug that could accidentally leak memory into the .url file
- ClientLocationPicker: add ability to allow picking files via new .allowFileSelection property
…targeting unavailable/detached drives and removes the respective policies

- adapt app code to SDK API changes
	- improved error reporting for when the target of a shortcut can't be accessed / found
	- simplified and generalized prompt for opening the URL a shortcut points to
…ShortcutResolution: add MDM option to allow all shortcuts, only URL shortcuts, only item shortcuts or no shortcuts
…root, the name for the shortcut was not derived correctly
- Localizable.strings: re-add strings auto-removed by git during rebase
@felix-schwarz
Copy link
Contributor Author

Personal Space and Shared with me

  • Each directory or file in the list has "status" icons that show users whether the file has already been
    users whether the file has already been shared, for example. Unfortunately
    these icons are not displayed by screen readers, so this information is not accessible to
    not accessible to screen reader users. -> every of this images is spelt together with "Actions available" during dictation. But, those icons are only indicators about the file status and no action is available by clicking on them or any other gesture. is that right @felix-schwarz ?

@jesmrec As far as I can see, the actions are inherited from the parent views / accessibility elements and can't be removed by the children:

Bildschirmfoto 2024-05-13 um 22 42 01

Triggering these actions triggers the respective actions of the parent views.

When I let Accessibility Inspector mimic VoiceOver by reading the description, it only read "Shared by link, image". What needs to be done to get VoiceOver to also say "Actions available"?

Image Viewer

  • When focusing on the image with the screen reader, the description of the image is not displayed.
    image, but information such as size and date entries. -> i don't understand what's going on here. What does "description of the image" mean? afaik, we don't describe images, not sure how to check this one

The file viewer shows general file information while loading the file (or for files that don't exist locally and can't be downloaded for f.ex. lack of connection). Once the file is loaded, that information is covered by the viewed file.

Previously, that file information was still read out loud because it was still part of the view hierarchy. Now, after the fix, it is no longer read out.

@jesmrec
Copy link
Contributor

jesmrec commented May 14, 2024

When I let Accessibility Inspector mimic VoiceOver by reading the description, it only read "Shared by link, image". What needs to be done to get VoiceOver to also say "Actions available"?

Just clicking on the items, Voice Over spells:

  • "Available offline image. Actions available"
  • "Shared image. Actions available"
  • "Shared by link image. Actions available"

Mi device is a iPhone XR iOS17.4.1

The file viewer shows general file information while loading the file (or for files that don't exist locally and can't be downloaded for f.ex. lack of connection). Once the file is loaded, that information is covered by the viewed file. Previously, that file information was still read out loud because it was still part of the view hierarchy. Now, after the fix, it is no longer read out.

Thanks for clarifying!

- ios-sdk: use branch fix/warnings that fixes OpenSSL warnings
- replace associated keys using structs or strings with static malloc results as suggested by Apple Engineer Quinn (https://forums.swift.org/t/handling-the-new-forming-unsaferawpointer-warning/65523/7)
- remove unneeded/unused overrides
- reformat spacing in parts of the code and remove superfluous spaces to remove SwiftLint warnings
- replace calls to CLLocationManager.authorizationStatus() with CLLocationManager.authorizationStatus
- remove deprecated OpenSSL ERR_load_* calls, since OpenSSL loads these automatically (https://www.openssl.org/docs/man3.3/man7/migration_guide.html)
- LAContext+Extension: add cases for Optic ID
- remove unused variables and calls
- remove unused, non-existent "gear.png" image from asset catalog
…es a summary of an array of items usable as an accessibilityLabel

- OCItem+Extension: add .accessibilityDateFormatter and .lastModifiedLocalizedAccessible accessors for formatting dates in formats that can be read by VoiceOver (fixing issue 5 in #1341)
- UniversalItemListCell:
	- from an accessibility perspective: replace building blocks that make up the cell content with a single accessibility element containing the complete information (fixing issue 2 in #1341)
	- make the more button NOT available as accessibility element so that item lists can be browsed fluently in VoiceOver. Where users want to access the more button, they can do so via the custom accessibility actions, which include the more button. When the cell is selected, swiping up and down allows picking an action, including "More".
- OCItem+UniversalItemListCellContentProvider: add accessibility labels for use by UniversalItemListCell and SegmentViewItem
- add localizations for new strings
	- add button trait (fixes #1341 sidebar issue)
	- fix warning, replace random number with UUID (even better)
- DriveGridCell: add role, use name of space as accessibility label, subtitle as accessibility hint and offer additional actions via accessibility custom actions.
- DriveListCell: add title for "moreAction" so that it can be converted to a accessibility custom action (which require a non-"" title)
- SavedSearchCell: consolidate accessibility elements, provide consolidated accessibility label derived from title and detail segment content
- OCShare+UniversalItemListCellContentProvider: provide VoiceOver-readable expiration date for links as accessibilityLabel (fixes #1341 issue)
- PopupButtonController: set accessibility label based on title so VoiceOver doesn't read out the rich-text image that is used
- SortBar: change accessibility label for multiselect button depending on status (fixes issue 3 in #1341)
- ActionCell: make cell a proper accessibility element (fixes issue 4 in #1341)
- ClientItemViewController: fix issue 4 in #1341
@felix-schwarz
Copy link
Contributor Author

@jesmrec The item cells now provide a consolidated description for VoiceOver, fixing that issue. I also addressed all issues in #1336, #1337, #1338 and #1341, which are now ready to be checked.

@felix-schwarz felix-schwarz linked an issue May 23, 2024 that may be closed by this pull request
12 tasks
@delete-merged-branch delete-merged-branch bot deleted the branch master May 27, 2024 13:06
@felix-schwarz felix-schwarz changed the base branch from milestone/12.3 to master May 27, 2024 21:12
@jesmrec
Copy link
Contributor

jesmrec commented May 28, 2024

#1341

Personal Space

  • When the elements on the page are sorted, the status of the sorting is
    sorting is only shown visually by arrows. However, this visual representation
    a limitation for non-visual users, however, as they do not receive any
    information about the current sorting.
  • Both directory and file names are buttons and can be activated.
    be activated. Unfortunately, these elements lack a corresponding role to
    their function as buttons appropriately.
  • Although the "Start multiple selection" button has an accessible name
    name, but this always remains the same, regardless of whether the multiple
    multiple selection has been activated or not. It would be advisable
    either change the accessible name or display the status of the multiple
    multiple selection status.
  • In multiple selection mode, further buttons appear at the bottom of the page.
    buttons appear at the bottom of the page. Unfortunately, these buttons have no role and there is no hint explaining their function.
  • Both date and time information are displayed unformatted and
    unformatted and incomprehensible. Further details can be found at
    https://a11y-guidelines.orange.com/en/mobile/ios/development/#date-timeand-numbers.

Menu

  • The account URL appears to be a button, but it has not been implemented as such
    with a corresponding role.
  • The "Personal" button has no role.
  • All submenus, such as "Shared with me", have no role.
  • The "Spaces" button does not have a role and informs users that it is a
    it is a collapsible element, but this is not the case.

Share with me

  • Both date and time information are output unformatted and
    incomprehensible - see comment on page 2.
  • Buttons that are located in the "Accepted" area, such as directory names
    have no role.

Spaces

  • The Space buttons do not have an assigned role and the
    hint is also missing.

@jesmrec
Copy link
Contributor

jesmrec commented May 29, 2024

#1337

Personal Space

  • In the test, it was possible to focus non-interactive elements with the keyboard,
    which should be avoided.

Shared with me

  • In the test, it was possible to focus the heading "Shared with me" with the keyboard, which should be avoided.
    which should be avoided.

Shared with others

  • In the test, it was possible to focus the heading "Shared with others" with the keyboard, which should be avoided.
    which should be avoided.

Spaces

  • In the test, it was possible to focus on the "Spaces" heading using the keyboard, which
    should be avoided.

Add Account

  • In the test, it was possible to focus the "Add account" heading with the keyboard, which should be avoided.
    which should be avoided.

Image Viewer

  • In the test, it was possible to focus the heading with the file name using the keyboard.
    which should be avoided.

@jesmrec
Copy link
Contributor

jesmrec commented May 29, 2024

#1338

Personal Space

  • For all buttons where the keyboard focus has been implemented by changing the background
    background color, the keyboard focus is difficult to see. The
    contrast between the focused and unfocused state is significantly below the recommended
    the recommended contrast ratio of 3:1.

Menu, Shared with me, Shared with others

  • See commentary on Personal Space.

Spaces

  • The space buttons have a barely visible keyboard focus.

@jesmrec
Copy link
Contributor

jesmrec commented May 29, 2024

#1336

Personal Space

  • In the test, it was not possible to focus the sort button with the keyboard.
    focus with the keyboard.

  • It is not possible to focus the "More" buttons using the keyboard.

Menu

  • The "Eject" button cannot be focused using the keyboard.

Shared with me

  • It is not possible to use the keyboard to focus the "More" buttons.

  • It is not possible to use the keyboard to focus the "Accept" and "Reject" buttons.
    "Reject" buttons with the keyboard.

Shared with others

  • The buttons with the arrow pointing to the right cannot be focused and activated using the keyboard.
    and activated with the keyboard. Instead, the entire list element is focused, but it
    cannot be activated.

Image Viewer

  • It is not possible to change the elements of the carousel using the keyboard.
    using the keyboard. It is only possible using swipe gestures.

@jesmrec
Copy link
Contributor

jesmrec commented May 29, 2024

About #1336, keyboard support. Some findings to comment:

(1)

The Drag Item option in the Tab + Z menu is available in many places, it makes sense only in list of files, right?. Places like the list of accepted shares in Shared with me, Shared by link or also in the options of the Sharing menu. In all of those places, it does not have an action. Check the following videos as example:

https://github.com/owncloud/ios-app/assets/14894746/ed187192-1ef5-47ea-ac2d-300734b60da5
https://github.com/owncloud/ios-app/assets/14894746/c0c1cd5f-ba6f-4d17-bac6-2e4707dad879

(2)

In the first video above, it's also visible a glitch in the Tab + Z menu

  1. In Share with me , select Decline
  2. When the item is declined, then select Accepted
  3. When is accepted again, open the Tab + Z menu

Current: Both Accept and Decline available
Expected: Only Decline available

(3)

Not sure if this one is in the scope, because the Sharing options were not included in the accessibility reports:

It's not posible to generate a password for a link, or setting an expiration date. Options not reachable with the keyboard.

(4)

Breadcrumbs in the bottom side of the file list are not browsable. The breadcrumb is reachable but the items inside the path.

Is it intended? feasible to fix?

iPhone XR iOS 17.5
f4ddd54e3

	- add new .accessibilityCustomAction action location for actions that should be available directly via custom actions in keyboard control and VoiceOver
	- add .accessibilityCustomAction to all applicable actions
	- add new provideAccessibilityCustomAction() method for creating accessibility custom actions for Actions
- AvailableOfflineAction: provide different name for custom actions based on status
- OCItem+ UniversalItemListCellContentProvider: make Actions available as accessibility custom actions
- UniversalItemListCell:
	- extend Content with accessibilityLabel and accessibilityCustomActionsBlock
	- add support for accessibilityLabel and accessibilityCustomActionsBlock
- CollectionViewController: add dragInteractionEnabled property to allow disabling dragging of items, so keyboard control does not include "Drag Item" in the accessibility actions invoked with Tab + Z
- ShareViewController + SharingViewController:
	- disable dragging via dragInteractionEnabled
	- provide accessibility labels and control for setting password and expiration date
- ClientShared*ViewController:
	- disable dragging via dragInteractionEnabled
@felix-schwarz
Copy link
Contributor Author

@jesmrec Thanks for the detailed feedback and the videos! Regarding your findings:

(1) is fixed.
(2) I can't reproduce this, but it's what would happen if an item is in OC10 state "pending". Did you encounter this issue with ocis or OC10?
(3) is also fixed:

  • setting the expiration date can now be done simply by selection (space bar) of the element and via custom actions. Custom actions also include extending and shortening the expiration date by one week.
  • likewise, all password actions are also available as custom actions - and the user can jump to the password editor by selection (space bar).
  • these options only appear once you've selected a role (via spacebar)
    (4) breadcrumbs should be keyboard-selectable IF they can actually be "pressed", f.ex. if you're in "Personal > Pictures", only "Personal" can be selected, and "Pictures" can't because you're already in that folder.

I've also brought the availability of actions on par with the Files.app, making the actions that were previously only available under Actions directly available as custom actions:
Simulator Screenshot - iPad Pro 13-inch (M4) - 2024-05-31 at 17 41 44

@jesmrec
Copy link
Contributor

jesmrec commented Jun 3, 2024

  • In Share by Link menu from sidebar, now Copy is missing, not reachable. Should be added tp the actions?

IMG_2390

  • Drag item was removed from many places, but i found it in list of spaces, in every focused space after Tab + Z

In oCIS, it may not be the only way to reproduce, if you try to decline - accept in some way, i t always appears. I have been able to reproduce just listing an item that was just shared (in this case, a duplicated declined option):

RPReplay_Final1717411070.MP4
  • Solution to password and expiration date is cool 💯
  • Breadcrumbs are also OK ✅
  • Files App OK ✅

#1349, making superfluous "Drag item" accessibility action no longer available for Spaces

- UniversalItemListCell: add support to automatically create and offer accessibility custom actions for button accessories
- OCShare+Interactions: make .offerAcceptAction and .offerDeclineAction available within the framework, make .offerDeclineAction more specific
- OCShare+UniversalItemListCellContentProvider:
	- take advantage of new UniversalItemListCell feature to provide an accessibility custom actions for "Copy to clipboard" for links (fix finding by @jesmrec in #1349) and to no longer offer duplicate actions that are already available via Swipe actions
	- switch to OCShare.offerAcceptAction and OCShare.offerDeclineAction to determine whether to show "Accept" or "Reject"/"Unshare" buttons
- code consolidation and cleanup
@felix-schwarz
Copy link
Contributor Author

@jesmrec The remaining issues should all be fixed now as of 6677219. Please have another look.

@jesmrec
Copy link
Contributor

jesmrec commented Jun 7, 2024

@felix-schwarz all the findings above are fixed

Unfortunately, i found more unexpected appearances of Drag Item:

  • In items inside the Locations section of Available Offline shortcut in sidebar
  • In Search section of sidebar, every item in Quick Access

These two should be the latest ones before finishing 😅

@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Jun 10, 2024

@jesmrec

@felix-schwarz all the findings above are fixed

🥳

Unfortunately, i found more unexpected appearances of Drag Item:

  • In items inside the Locations section of Available Offline shortcut in sidebar

Unfortunately, Apple does not currently provide API support for disabling dragging - from an Accessiblity perspective - for individual items or sections, just for the entire collection view / table view.

For actual touch interactions, there is an API, but it is not consulted by iOS when putting the list of actions together for Tab + Z.

It's therefore not possible to remove the Drag item action from the Locations part without also removing it from the Downloaded files part, which I'd consider a bigger loss than what is won removing that action from the Locations part.

  • In Search section of sidebar, every item in Quick Access

Fixed with the latest commit.

@jesmrec
Copy link
Contributor

jesmrec commented Jun 10, 2024

Fixed the latest commit, and approved on my side

@TheOneRing TheOneRing merged commit 885dd57 into master Jun 11, 2024
3 of 4 checks passed
@TheOneRing TheOneRing deleted the fix/a11y branch June 11, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants