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

Red field outlines are not displayed when entering incorrect information #4730

Closed
isagoico opened this issue Aug 18, 2021 · 22 comments
Closed
Assignees

Comments

@isagoico
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue is failing #4431

Action Performed:

  1. Navigate to /bank-account/company
  2. Enter incorrect information in the text fields.
  3. Submit the form.

Expected Result:

When trying to submit the form, the incorrect field should be highlighted in red.

Actual Result:

No red outline is seen in the incorrect fields after trying to submit the form.

Platform:

Where is this issue occurring?

  • Web

Version Number: 1.0.86-2

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

image

Bug5197900_Recording__292.mp4

Expensify/Expensify Issue URL:

View all open jobs on GitHub

@isagoico isagoico added DeployBlockerCash This issue or pull request should block deployment Engineering Daily KSv2 labels Aug 18, 2021
@MelvinBot
Copy link

Triggered auto assignment to @aldo-expensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@aldo-expensify
Copy link
Contributor

Working on this...

@aldo-expensify
Copy link
Contributor

I can see in the render method of <CompanyStep> that we pass down the error text like <ExpensiTextInput errorText={error} ...>, then this eventually uses <BaseExpensiTextInput> to render the input. <BaseExpensiTextInput> receives the errorText with the error, but this component doesn't handle this prop.

I saw that there is another component that actually does something with this prop: <TextInputWithLabel>

@aldo-expensify
Copy link
Contributor

PR up, I tested in desktop.. seems to work, I'm testing in other platforms now.

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Aug 18, 2021

How can we get to this view from the Desktop/Mobile native versions? Follow flow VBA flow: Add Bank account, then company.

@kevinksullivan
Copy link
Contributor

@aldo-expensify should this have closed?

@kevinksullivan
Copy link
Contributor

How can we get to this view from the Desktop/Mobile native versions? Follow flow VBA flow: Add Bank account, then company.

You should be able to get to this step on mobile/desktop using the same information in (3) of this SO

https://stackoverflow.com/c/expensify/questions/342

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Aug 23, 2021

@aldo-expensify should this have closed?

Hey @kevinksullivan , I think this should be closed, the problem described in this issue was fixed in the PR: #4748

I would suggest opening a new issue/pr if we wanted to work on the show all error messages at the same time stuff.

@kevinksullivan
Copy link
Contributor

but the relevant PR is not deployed, correct @aldo-expensify ?

#4741

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Aug 23, 2021

but the relevant PR is not deployed, correct @aldo-expensify ?

#4741

oh, it has not been deployed , you are right

@isagoico
Copy link
Author

Reopening this since we found an issue when retesting the fix #4741 (comment)

@isagoico isagoico reopened this Aug 23, 2021
@kavimuru

This comment has been minimized.

@marcaaron
Copy link
Contributor

@aldo-expensify has brought to our attention that the multiple errors should not be expected yet. We are still implementing it so we can remove the deploy blocker label.

@marcaaron marcaaron reopened this Aug 24, 2021
@marcaaron marcaaron removed the DeployBlockerCash This issue or pull request should block deployment label Aug 24, 2021
@aldo-expensify
Copy link
Contributor

aldo-expensify commented Aug 24, 2021

I'll investigate what's the problem with the android version, it is not working there, right? Or at least that is what I understand from this: #4741 (comment)

@marcaaron
Copy link
Contributor

Ok I think this is a blocker actually after talking to @aldo-expensify. 😂

Android is failing for some reason so we must figure that out first ?
https://github.com/Expensify/App/runs/3403823191?check_suite_focus=true#step:12:1483

This comment is not a blocker and is being handled in https://github.com/Expensify/Expensify/issues/174749
#4730 (comment)

@aldo-expensify
Copy link
Contributor

I tested this in an android emulator and found this incorrect behaviour:

Screen.Recording.2021-08-23.at.7.04.19.PM.mov

Basically, the value of the field with the error disappears, but once we focus it again, the value appears again. Would be good to know if this happens in a real android device.

@marcaaron
Copy link
Contributor

Yes that is weird. The value of this field should not disappear at all, but is it a regression?

@aldo-expensify
Copy link
Contributor

Yes that is weird. The value of this field should not disappear at all, but is it a regression?

I would say it is a regression. In the production app we don't have this red outline with the red message under the field, you just have the growl, and the value of the field doesn't disappear

I just tried it on an android device, and the behaviour is the same (the value disappearing until I touch the field).

@aldo-expensify
Copy link
Contributor

There seem to be something with the size of the actual text input, I can scroll inside the "input" and then I see the value.

@botify
Copy link

botify commented Aug 30, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

1 similar comment
@botify
Copy link

botify commented Aug 30, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

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

No branches or pull requests

8 participants