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

Mobile click accessibility #11447

Merged
merged 3 commits into from
Jan 31, 2022
Merged

Mobile click accessibility #11447

merged 3 commits into from
Jan 31, 2022

Conversation

steverep
Copy link
Member

Proposed change

Change the Polymer setting cancelSyntheticClickEvents to false globally in the UI. When left to true, it ends up breaking nearly all buttons in the UI for mobile screen readers.

PS - I know the 2nd commit is only loosely related, but that comment would have saved me a bunch of time trying to figure out why putting the code there wasn't working

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

N/A - Just flip VoiceOver or Talkback on with and without this change.

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

@steverep
Copy link
Member Author

Sorry, forgot to mention that I have no custom panel to test, but the 2 lines of code are identical for each entrypoint.

Also, I assume there is no need to add it to the core.ts entrypoint as no UI elements go through there? And did I miss any edge points that I may not have thought to test?

@bramkragten
Copy link
Member

Thanks for this PR! I'll dive into this tomorrow

@steverep
Copy link
Member Author

This should also close #11034 which seems like a duplicate issue.

@zsarnett zsarnett linked an issue Jan 28, 2022 that may be closed by this pull request
3 tasks
@steverep
Copy link
Member Author

Any chance this can get in the upcoming Feb release? 🥺

@bramkragten
Copy link
Member

Any chance this can get in the upcoming Feb release? 🥺

I don't think that is wise, I would like it to go through a complete beta and some testing before shipping this into production. We are not sure if this will break anything...

@steverep
Copy link
Member Author

I don't think that is wise, I would like it to go through a complete beta and some testing before shipping this into production. We are not sure if this will break anything.

I understand the hesitation on a global setting, but...

  • It's already broken! This is absolutely debilitating to any screen reader user. There is no work around. The mobile apps continue to be useless without this fix.
  • The vast majority of components, including all mwc-* and nearly all paper-*, don't use the problematic module where this setting is employed. The only reason they are affected is because of reckless and intrusive coding IMHO. See the polymer issue I opened.
  • The only documentation I could find on this setting is the single comment where it is declared, and that more or less acknowledges it is not needed and has detrimental effects:
* Setting to cancel synthetic click events fired by older mobile browsers. Modern browsers
 * no longer fire synthetic click events, and the cancellation behavior can interfere
 * when programmatically clicking on elements.
 */
export let cancelSyntheticClickEvents = true;

If you still think extensive testing is necessary, here are the only 9 modules in the entire source tree that actually import the offending gestures.js. It would not be difficult to quickly test where they are actually used in the frontend (if at all).

  • node_modules/@polymer/iron-overlay-behavior/iron-overlay-manager.js:
    • 16: import * as gestures from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@polymer/paper-dropdown-menu/paper-dropdown-menu-light.js:
    • 27: import * as gestures from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@polymer/paper-dropdown-menu/paper-dropdown-menu.js:
    • 28: import * as gestures from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@polymer/paper-slider/paper-slider.js:
    • 21: import {setTouchAction} from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@polymer/polymer/lib/legacy/legacy-element-mixin.js:
    • 18: import { setTouchAction } from '../utils/gestures.js';
  • node_modules/@polymer/polymer/lib/mixins/gesture-event-listeners.js:
    • 13: import { addListener, removeListener } from '../utils/gestures.js';
  • node_modules/@vaadin/vaadin-button/src/vaadin-button.js:
    • 8: import { addListener } from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@vaadin/vaadin-date-picker/src/vaadin-date-picker-mixin.js:
    • 9: import { addListener } from '@polymer/polymer/lib/utils/gestures.js';
  • node_modules/@vaadin/vaadin-date-picker/src/vaadin-date-picker-overlay-content.js:
    • 8: import { addListener, setTouchAction } from '@polymer/polymer/lib/utils/gestures.js';

@bramkragten
Copy link
Member

I'm not worried about issues with current browsers but with the old browsers, like mentioned in that comment.

I'm not sure what devices/browsers are affected.

I guess we can put this in, as the beta generally doesn't find issues with old browsers anyway...

@bramkragten bramkragten merged commit fb55ab1 into home-assistant:dev Jan 31, 2022
@steverep steverep deleted the mobile-click-a11y branch February 1, 2022 02:40
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2022
@bramkragten
Copy link
Member

paper-dropdown-menu seems not to work anymore on iOS, I propose we revert this PR for now, and work on replacing all paper-dropdown-menu with mwc-select. After that we can enable this again (next release?).

@bramkragten
Copy link
Member

In the meantime if needed, people could just stay on version 2022.2.0 until we have fixed this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants