[$250] [Wave Collect][QBO] Refactor the error text style

[hayata-suenaga]

From this convo in another App PR.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ab49e87b72a1e37d
• Upwork Job ID: 1795779159359643648
• Last Price Increase: 2024-05-29
• Automatic offers:
• s77rt | Contributor | 102509287

Issue Owner

Current Issue Owner: @laurenreidexpensify

[narefyev91]

I will pick this one - can you please assign this to me @hayata-suenaga

[melvin-bot]

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

[narefyev91]

@shawnborton can you please clarify - are we expecting Error Text position like this one?

[shawnborton]

That looks good to me, cc @Expensify/design for thoughts too.

What does the :hover style look like? Just want to confirm that the whole thing is included in the hoverable area (push row + error text)

[shawnborton]

While we are at it, I wonder if we should take a look at the hint text too to make sure the hint text works the same way - same distance below the push row, and is included in the hoverable interaction.

[narefyev91]

@shawnborton hover:

[narefyev91]

Yeah for sure - i will also include hint issue to this PR fix

[shawnborton]

Okay cool, as it stands now, it seems like maybe there is too much padding below the error text? Notice how the bottom portion of the hover is larger than the top.

[narefyev91]

Okay cool, as it stands now, it seems like maybe there is too much padding below the error text? Notice how the bottom portion of the hover is larger than the top.

Yeah - this coming from standard menu item component. - should we also change paddings which we already have?

[shawnborton]

Yeah, I would expect that vertical padding to be equal. So if we're doing 12px on top, why not 12px on the bottom? Or maybe we just do 16px vertically so we split the difference?

[narefyev91]

Yeah, I would expect that vertical padding to be equal. So if we're doing 12px on top, why not 12px on the bottom? Or maybe we just do 16px vertically so we split the difference?

Good point - but i do not see any vertical logic to make a difference.
In any case - it should be 12px below - based on all other menu items. I will update that for sure! Thanks!

[dannymcclain]

Just popping in to say I agree with everything going on in this convo.

[narefyev91]

@shawnborton this one with hint text below:

[shawnborton]

Nice, and can you show us that same scenario but with no error text?

Also, in the error case, is it possible to move the red dot closer to the right arrow? They feel a little far away right now, cc @Expensify/design for a gut check on that one. Perhaps we can try 8px and 12px gaps, respectively, for comparison?

[narefyev91]

@shawnborton here you are:

[narefyev91]

And closer red dot as well:

[shawnborton]

Nice, both of those make sense to me. The last small thing I see is that the line height for the error text seems different from the line height we're using for the hint text. I think at the label font size (13px), we use 16px for the line height. Can you double check that for me? Thanks!

[narefyev91]

Nice, both of those make sense to me. The last small thing I see is that the line height for the error text seems different from the line height we're using for the hint text. I think at the label font size (13px), we use 16px for the line height. Can you double check that for me? Thanks!

yeah - for error it is 18px - should we make it 16px as well?

[shawnborton]

Yes please! Not sure how 18px ever got introduced as a line height, I don't think we should be using it anywhere?

[narefyev91]

Yes please! Not sure how 18px ever got introduced as a line height, I don't think we should be using it anywhere?

Nice - sure will update that!

[dannymcclain]

Yeah this is looking good!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (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.

[hayata-suenaga]

Payment for the C+ role for @s77rt is needed for this App PR.

The PR caused a regression reported here.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[laurenreidexpensify]

@s77rt please accept role in Upwork and will issue payment thanks

[s77rt]

@laurenreidexpensify Accepted! Please note this is due $125 as there was a regression

[laurenreidexpensify]

Payment Summary:

C+ Review $125 @s77rt paid in upwork