Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: updated field multiline to have enough width #2208

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

btw17
Copy link
Member

@btw17 btw17 commented Feb 17, 2024

The basics

The details

Resolves

Fixes #2095

Proposed Changes

Updated the width to have 1px extra on each side.

Reason for Changes

From what I can tell, this issue is created by rounding in the browser in different contexts. With Zoom = 100%, this worked as intended for me before the fix. Zooming in, though, I noticed the calculated component ended up as ?.06 and the text area ended up as ?.05. Essentially, there's barely not enough space for the content, so it wraps to the next line. (See screenshots below.)

Test Coverage

No unit tests needed, though here's what I manually tested:

  • Various zoom levels (from <100% to >100%)
  • Various string lengths
    • Note: See my comment in Additional Information.

Before at 125% zoom: (Note: the last character wraps to a new line)
Screenshot 2024-02-16 6 50 47 PM

After at 125% zoom:
Screenshot 2024-02-16 7 05 17 PM

Documentation

N/A

Additional Information

What's the expected behavior when the text is long enough to produce an ellipsis? It seems like it wrapping to a new line when the text is too wide in this context might be expected.

Long text with "...":
5G9LHa25gupstxd

Corresponding long input text wrapping:
AH3zxgiiWrkyNSK

@btw17 btw17 requested a review from a team as a code owner February 17, 2024 01:07
@btw17 btw17 requested review from cpcallen and removed request for a team February 17, 2024 01:07
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

LGTM in present form or with order-of-operations change you proposed in standup.

@btw17
Copy link
Member Author

btw17 commented Feb 23, 2024

Slight tweak I meant to get out - instead of adding +2px, just needs to add +1px. Should be good to go now, though @cpcallen! When you're good with it, you'll need to be the one to merge this in.

@btw17 btw17 force-pushed the fix_field_multiline_wrapping branch from bdbd611 to 1353936 Compare February 23, 2024 19:31
@cpcallen cpcallen merged commit 376691d into google:master Feb 26, 2024
8 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Field multiline rendering incorrectly for long inputs.
2 participants