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

Refactor TextInputWithFocusStyles with TextInput #7721

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Feb 13, 2022

Details

  • TextInputWithFocusStyles

    Purpose of component

    1. This is just used to change some styles when input is in a focused state and vice-versa.

    Remove the extra component and use TextInput instead.

Fixed Issues

$ #6584
$ #7309

Tests | QA Steps

  1. Open Request Money page.
  2. Click Currency.
  3. Check that you are able to search the currency from the search bar.
  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web | Desktop

image

Mobile Web

iOS

image

Android

image

@parasharrajat parasharrajat changed the title Merge Refactor TextInputWithFocusStyles with TextInput Feb 13, 2022
@parasharrajat parasharrajat marked this pull request as ready for review February 14, 2022 16:07
@parasharrajat parasharrajat requested a review from a team as a code owner February 14, 2022 16:07
@MelvinBot MelvinBot requested review from puneetlath and rushatgabhane and removed request for a team, puneetlath and rushatgabhane February 14, 2022 16:07
Copy link
Member

@rushatgabhane rushatgabhane left a comment

Choose a reason for hiding this comment

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

@puneetlath LGTM! Tests well on all platforms

@puneetlath puneetlath merged commit d0e26aa into Expensify:main Feb 15, 2022
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @puneetlath in version: 1.1.40-0 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Mar 1, 2022

🚀 Deployed to production by @yuwenmemon in version: 1.1.40-2 🚀

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

@Stutikuls
Copy link

Stutikuls commented Mar 9, 2022

Issue 1 -

Title- [Medium]: Chrome+ Jaws : Screen reader : Role is not defined for 'Indian Rupee sign icon' control.
Actual - Focus lands on the 'Indian Rupee sign icon' control then screen reader is reading "Indian Rupee sign".
Expected - Role = 'Button' should be defined for Indian Rupee sign icon control.
WCAG failure - 4.1.2
Reproducible in staging?: - Yes
Version Number: - v1.1.40-2
Platforms - Web (Chrome + Jaws), iOS, Mobile- web(iOS)

7721_Not.reading.the.role.for.the.icon.mp4

Issue 2 -

Title- [Medium]: Chrome+ Jaws : Screen reader : Screen reader is not provide information about the search result .
Actual - Screen reader is silent after finding the search results.
Expected -After finding the search results screen reader should provide the search results information like "1 result found".
WCAG failure - 4.1.3
Reproducible in staging?: - Yes
Version Number: - v1.1.40-2
Platforms - Web (Chrome + Jaws), iOS, Mobile- web(iOS), Desktop (MAC - Safari + Voiceover). Android (Talkback)

7721_Not.provide.the.search.result.information.mp4

Issue 3 -

Title- [High]: Safari + Voiceover : Keyboard : 'Indian rupees icon' control is not accessible using Swipe gesture.
Actual - 'Indian rupees icon' control is not accessible using Swipe gesture, focus does not land on the 'Indian rupees icon' control.
Expected -'Indian rupees icon' control should accessible using Swipe gesture.
WCAG failure - 2.1.1
Reproducible in staging?: - Yes
Version Number: - v1.1.40-2
Platforms - Mobile- web(iOS)

7721_Rupees.icon.is.not.accessible.using.swipe.gesture.mp4

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.

5 participants