Fix file picker truncating paths within available width#9885
Fix file picker truncating paths within available width#9885vorporeal merged 1 commit intowarpdotdev:masterfrom
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Brad Reynolds.
|
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @vorporeal. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR removes the command palette file picker's fixed combined name/path cap, changes overflowed path text to leading ellipsis clipping, and adjusts start-ellipsis glyph painting in WarpUI.
Concerns
- The new unit test documents that the test FontDB gives the ellipsis a zero-width advance, so it does not exercise the newly added nonzero ellipsis offset path.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| &mut scene, | ||
| ); | ||
|
|
||
| // The platform test FontDB returns `glyph_advance == 0` for the |
There was a problem hiding this comment.
💡 [SUGGESTION] Because the test FontDB makes ellipsis_width zero, this regression test never executes the new start_ellipsis_offset = ellipsis_width path; use a nonzero ellipsis advance or a smaller unit seam so it fails without the production fix.
vorporeal
left a comment
There was a problem hiding this comment.
looks way better. thanks for the fix!
|
@bradleyjames you'll need to modify your commit (and re-push it) so the author information matches your github account, and then sign our CLA - this is a requirement for us to accept contributions. you can use the following to do so: where [email protected] is an email address associated with your GitHub account. |
The file picker capped combined filename + path length at 55 characters, leaving significant horizontal space unused in wider popovers. Drops that cap for the command palette file picker and switches the path text to render a leading `…` (instead of a fade) when overflow does happen. Also fixes a latent paint bug in `Start + Ellipsis` text clipping: the ellipsis-reservation shifted glyphs leftward without compensating their origin, so the leftmost visible glyph overlapped the ellipsis at the same x. Adds regression-protection unit tests for the start-clipping paint path. Fixes warpdotdev#8709
21c4293 to
085f8da
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @bradleyjames on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
@vorporeal , done |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
The file picker capped combined filename + path length at 55 characters, leaving significant horizontal space unused in wider popovers. Drops that cap for the command palette file picker and switches the path text to render a leading
…(instead of a fade) when overflow does happen.Also fixes a latent paint bug in
Start + Ellipsistext clipping: the ellipsis-reservation shifted glyphs leftward without compensating their origin, so the leftmost visible glyph overlapped the ellipsis at the same x. Adds regression-protection unit tests for the start-clipping paint path.Fixes #8709
Description
What is a design buddy?
Linked Issue
ready-to-specorready-to-implement.Screenshots / Videos
Testing
Look at the PR.
Agent Mode
Used claude code.