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

Only deactivate label if field isn't focused #5789

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Oct 12, 2021

Details

This PR fixes an issue where the Expensitext label value will move over the cursor after deleting the content but keeping focus. In the case that the input has a label and a placeholder, this looks especially bad (see https://github.com/Expensify/Expensify/issues/180809).

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/180809

Tests/QA

  1. Create a new account and create a workspace
  2. Open the workspace and click Issue Corporate Cards and then Connect Bank Account. Fill out anything for the values on Connect Bank Account and submit to get to the Company Information step
  3. Confirm that when you get to the Company Information step, the phone number placeholder doesn't overlap the label like in https://github.com/Expensify/Expensify/issues/180809 when you enter a value and then remove the value
  4. Confirm that the placeholders for all fields behave normally when clicking into and out of them

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

@Jag96 Jag96 self-assigned this Oct 12, 2021
@Jag96 Jag96 requested a review from a team as a code owner October 12, 2021 19:31
@MelvinBot MelvinBot requested review from mountiny and removed request for a team October 12, 2021 19:32
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.

Great job finding the fix so quickly. LGTM :shipit:

@mountiny
Copy link
Contributor

Feel free to self-merge once the E2E tests will pass and you will see it before me 😄

@Jag96 Jag96 merged commit 786223d into main Oct 12, 2021
@Jag96 Jag96 deleted the joe-deactivate-label branch October 12, 2021 20:33
@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 @Jag96 in version: 1.1.7-25 🚀

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

@isagoico
Copy link

I think this PR may be the culprit for this issue #5904

@ogumen
Copy link

ogumen commented Oct 22, 2021

Please note that the entry format for some fields is conveyed using placeholder only, and the placeholder text disappears once user enters any character into the field. It is mandatory to display the permanently visible instructions when user entry is required (WCAG SC 3.3.2). The issue is mentioned in #5703 (comment)

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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

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