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

[Hold for payment 2024-10-07][$250] Spacing in error message in Fields for Expensify Page are not consistent #48659

Closed
4 of 6 tasks
IuliiaHerets opened this issue Sep 5, 2024 · 36 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 5, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: v9.0.29-6
Reproducible in staging?: Y
Reproducible in production?: N
Issue was found when executing this PR: #47957
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

  1. Access staging.new.expensify.com and sign in with a valid account
  2. Open Settings > Switch to Expensify Classic
  3. Select any reason and proceed to the next screen
  4. Verify the next button is aligned at the bottom of the page
  5. Submit the form with an empty response. Verify error is shown
  6. Keep adding new line to the form input until it reaches the maximum height
  7. Verify the input underline, form error, and the next button isn't overlap

Expected Result:

User expects consistent spacing between the "red dot - error messages" on all platforms at the bottom of the page

Actual Result:

The spacing is inconsistent between platforms, for example the spacing shows one way on iOS/Desktop and with another spacing on Web/mweb.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6594231_1725553323941!Different_distances_between_error_messages

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021832083749373830381
  • Upwork Job ID: 1832083749373830381
  • Last Price Increase: 2024-09-13
  • Automatic offers:
    • alitoshmatov | Reviewer | 104008602
Issue OwnerCurrent Issue Owner: @alitoshmatov
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Sep 5, 2024
Copy link

melvin-bot bot commented Sep 5, 2024

Triggered auto assignment to @francoisl (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Sep 5, 2024

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 5, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 5, 2024
Copy link
Contributor

github-actions bot commented Sep 5, 2024

👋 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.

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@MuaazArshad
Copy link
Contributor

MuaazArshad commented Sep 5, 2024

Edited by proposal-police: This proposal was edited at 2024-09-05 18:13:57 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Spacing in error message in Fields for Expensify Page are not consistent

What is the root cause of that problem?

This is happening due to margin overlap.
Screenshot 2024-09-05 at 10 30 29 PM
Screenshot 2024-09-05 at 10 30 05 PM

Due to the different dimensions of the platforms, the margin overlap is different which is causing inconsistency.

What changes do you think we should make in order to solve the problem?

The only feasible solution here is to see maxAutoGrowHeight from responseInputMaxHeight to variables.textInputAutoGrowMaxHeight here

maxAutoGrowHeight={responseInputMaxHeight}

as in
maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight}

What alternative solutions did you explore? (Optional)

We can also solve this by setting the responseInputMaxHeight and removing some of the space for errors here

const responseInputMaxHeight = NumberUtils.roundDownToLargestMultiple(
formMaxHeight -
safeAreaInsetsBottomValue -
safePaddingBottomStyleValue -
formPaddingBottomValue -
// Minus the height of the text component
headerTitleHeight -
// Minus the response input margins (multiplied by 2 to create the effect of margins on top and bottom).
// marginBottom does not work in this case because the TextInput is in a ScrollView and will push the button beneath it out of view,
// so it's maxHeight is what dictates space between it and the button.
baseResponseInputContainerStyle.marginTop * 2 -
// Minus the approximate size of a default button
variables.componentSizeLarge -
// Minus the height above the button for the form error text, accounting for 2 lines max.
variables.lineHeightNormal * 2 -
// Minus the margin between the button and the form error text
styles.mb3.marginBottom,
// Round down to the largest number of full lines
styles.baseTextInput.lineHeight,
);

@francoisl francoisl added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Sep 5, 2024
@francoisl
Copy link
Contributor

Demoting for now, this is not breaking anything.


@MuaazArshad thanks for the proposal. Were you able to identify which PR from the checklist caused the regression?

@MuaazArshad
Copy link
Contributor

No, I was not able to identify which PR caused it.

@ahmedGaber93
Copy link
Contributor

It seems related to #47957

@lschurr lschurr added the External Added to denote the issue can be worked on by a contributor label Sep 6, 2024
@melvin-bot melvin-bot bot changed the title Spacing in error message in Fields for Expensify Page are not consistent [$250] Spacing in error message in Fields for Expensify Page are not consistent Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021832083749373830381

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

Copy link

melvin-bot bot commented Sep 9, 2024

@francoisl, @lschurr, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@trjExpensify
Copy link
Contributor

The linked PR is in the regression period, I think @bernhardoj & @rayane-djouah should look at this as a regression.

Copy link

melvin-bot bot commented Sep 13, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@francoisl
Copy link
Contributor

Yeah let's proceed with a fix.
@alitoshmatov can you review the existing proposal please? The proposed solution makes sense, though I can't tell if the proposed alternative is worth it.

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@MuaazArshad
Copy link
Contributor

@alitoshmatov bump!

Copy link

melvin-bot bot commented Sep 16, 2024

@francoisl, @lschurr, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

@lschurr
Copy link
Contributor

lschurr commented Sep 16, 2024

@alitoshmatov
Copy link
Contributor

Okay we can go with @MuaazArshad 's proposal which changes max height of response input making it smaller and not reach bottom, like this:

Screenshot 2024-09-17 at 10 10 49 PM

C+ reviewed 🎀 👀 🎀

@melvin-bot melvin-bot bot removed the Overdue label Sep 17, 2024
Copy link

melvin-bot bot commented Sep 17, 2024

Current assignee @francoisl is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 17, 2024
Copy link

melvin-bot bot commented Sep 17, 2024

📣 @alitoshmatov 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Sep 17, 2024

📣 @MuaazArshad You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@MuaazArshad
Copy link
Contributor

I'll raise PR by EOD.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 18, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Sep 18, 2024
@MuaazArshad
Copy link
Contributor

@lschurr I haven't got the Upwork offer for this issue.

@MuaazArshad
Copy link
Contributor

@lschurr bump for payment!

@alitoshmatov
Copy link
Contributor

PR went into production on 1st of October

@lschurr
Copy link
Contributor

lschurr commented Oct 7, 2024

@MuaazArshad - since the PR went to production on Oct 1, today is the pay day. Sent your offer via Upwork.

@lschurr lschurr changed the title [$250] Spacing in error message in Fields for Expensify Page are not consistent [Hold for payment 2024-10-07][$250] Spacing in error message in Fields for Expensify Page are not consistent Oct 7, 2024
@MuaazArshad
Copy link
Contributor

Thank you. Accepted the offer.

@lschurr
Copy link
Contributor

lschurr commented Oct 7, 2024

Payment summary:

@lschurr lschurr closed this as completed Oct 7, 2024
@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #wave-collect Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: Done
Development

No branches or pull requests

9 participants