Page MenuHomePhabricator

[web] fix user profile long username bug
ClosedPublic

Authored by ginsu on Oct 25 2023, 8:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 1:24 PM
Unknown Object (File)
Wed, Nov 13, 1:24 PM
Unknown Object (File)
Wed, Nov 13, 1:24 PM
Unknown Object (File)
Wed, Nov 13, 1:23 PM
Unknown Object (File)
Wed, Nov 13, 1:20 PM
Unknown Object (File)
Sat, Nov 9, 11:03 PM
Unknown Object (File)
Oct 11 2024, 8:05 PM
Unknown Object (File)
Oct 11 2024, 8:05 PM

Details

Summary

This diff fixes the bug where we had text overflow with longer usernames. To address this I introduced a version of SingleLine for web.

For context here is the native version of SingleLine
https://github.com/CommE2E/comm/blob/master/native/components/single-line.react.js

Linear task: https://linear.app/comm/issue/ENG-5372/fix-user-profiles-for-long-usernames

Test Plan

Please see screenshots below

Before:

Screenshot 2023-10-25 at 11.17.54 PM.png (1×3 px, 663 KB)

after:

Screenshot 2023-10-25 at 11.24.46 PM.png (1×3 px, 660 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
ginsu requested review of this revision.Oct 25 2023, 8:46 PM
atul added a subscriber: ted.
atul added inline comments.
web/components/single-line.react.js
11 ↗(On Diff #32418)

Calling this children is a little weird? I'd maybe expect a React.Node or something. We could just call it text or something along those lines?

22 ↗(On Diff #32418)

Thoughts on making this like

<div title={children} className={singleLineClassName}>{text}</div>

so user can see full username on hover?

CC @ashoat/@ted since I guess this is kind of a design decision

This revision is now accepted and ready to land.Oct 26 2023, 9:54 AM
web/components/single-line.react.js
11 ↗(On Diff #32418)

Spoke to @atul IRL and we decided to just keep it the same so that both SingleLine components in native and web have the same api

22 ↗(On Diff #32418)

Not sure if I am a big fan of this. Going to land this for now, but if this is a feature that we want I will create a follow up Linear task for it

ginsu edited the test plan for this revision. (Show Details)

rebase before landing

This revision was landed with ongoing or failed builds.Oct 26 2023, 11:31 AM
This revision was automatically updated to reflect the committed changes.