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

#3102 add accessibility role and label to close button #3351

Conversation

dklymenk
Copy link
Contributor

@dklymenk dklymenk commented Jun 3, 2021

Details

Simple addition off accessibility props.

Fixed Issues

Fixes #3102

Tests / QA Steps

  1. Turn on Screen Reader or Voiceover
  2. Launch the Expensify.cash app or website
  3. Swipe or click into any menu that has an X "close" button. For example:
    ( + ) > New Chat > X
    ( + ) > Request Money > X
    ( + ) > New Group > X
    tap avatar icon > X
    tap search mag icon > X
  4. Make sure that the X "Close" button has a role and a label.

Tested On

  • Web - Windows/JAWS/Firefox - MACOs/Voiceover/Safari
  • Mobile Web
  • Desktop - Voiceover
  • iOS - iOS/Voiceover
  • Android - Android/Talkback

I do not own a physical iPhone and there is no Voiceover on iOS simulator, so I am unable to test accessibility features on iOS app.

Screenshots

Web

win10.mp4
mac1.mp4

Mobile Web

mweb.mp4

Desktop

electron.mp4

iOS

Android

android.mp4

Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

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

Tested on Desktop, Web and tested in ios simulator using this approach although I am not sure if it is bulletproof. Unfortunately, I do not have Android phone and I do not have certificates et up yet to test it on my phone. I have requested the certificates.

@mountiny mountiny merged commit d534b5f into Expensify:main Jun 4, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Jun 4, 2021

🚀 Deployed to staging in version: 1.0.62-6🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Jun 8, 2021

🚀 Deployed to production in version: 1.0.64-0🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@iwiznia
Copy link
Contributor

iwiznia commented Jun 10, 2021

You added a new language key in this PR but you only added it to the en language. We need to add it to the es language too. Close in spanish is Cerrar, can you please submit a PR adding it @dklymenk?

@dklymenk
Copy link
Contributor Author

Ok, I'll submit a PR as soon as I can.

Meanwhile, you might also want to take a look at this MR - https://github.com/Expensify/Expensify.cash/pull/3285/files

It also makes accessibility changes and adds translations for EN only.

@iwiznia
Copy link
Contributor

iwiznia commented Jun 10, 2021

Yep, thanks for that link!

@dklymenk
Copy link
Contributor Author

@iwiznia Please find PR with missing translation here #3525

@iwiznia
Copy link
Contributor

iwiznia commented Jun 10, 2021

Thanks! I approved it already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility improvement: Add a role, label, and voiceover to the "close" X button
5 participants