Fix file picker truncating paths within available width#9885
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@address.com 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 + 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 ## Description <!-- Please remember to add your design buddy onto the PR for review, if it contains any UI changes! --> What is a design buddy? ## Linked Issue <!-- Link the GitHub issue this PR addresses. Before opening this PR, please confirm: --> - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [X] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos <img width="1774" height="1326" alt="Screenshot_redacted_2" src="https://github.com/user-attachments/assets/596bb770-d64f-443c-9da9-30e4fe0bf727" /> ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? --> Look at the PR. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Used claude code. <!-- ## Changelog Entries for Stable The entries below will be used when constructing a soft-copy of the stable release changelog. Leave blank or remove the lines if no entry in the stable changelog is needed. Entries should be on the same line, without the `{{` `}}` brackets. You can use multiple lines, even of the same type. The valid suffixes are: * NEW-FEATURE: for new, relatively sizable features. Features listed here will likely have docs / social media posts / marketing launches associated with them, so use sparingly. * IMPROVEMENT: for new functionality of existing features. * BUG-FIX: for fixes related to known bugs or regressions. * IMAGE: the image specified by the URL (hosted on GCP) will be added to Dev & Preview releases. For Stable releases, see the pinned doc in the #release Slack channel. * OZ: Oz-related updates. Use `CHANGELOG-OZ`. At most 4 Oz updates are shown in-app per release. CHANGELOG-NEW-FEATURE: {{text goes here...}} CHANGELOG-IMPROVEMENT: {{text goes here...}} CHANGELOG-BUG-FIX: {{text goes here...}} CHANGELOG-BUG-FIX: {{more text goes here...}} CHANGELOG-IMAGE: {{GCP-hosted URL goes here...}} CHANGELOG-OZ: {{text goes here...}} -->
) 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 ## Description <!-- Please remember to add your design buddy onto the PR for review, if it contains any UI changes! --> What is a design buddy? ## Linked Issue <!-- Link the GitHub issue this PR addresses. Before opening this PR, please confirm: --> - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [X] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos <img width="1774" height="1326" alt="Screenshot_redacted_2" src="https://github.com/user-attachments/assets/596bb770-d64f-443c-9da9-30e4fe0bf727" /> ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? --> Look at the PR. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Used claude code. <!-- ## Changelog Entries for Stable The entries below will be used when constructing a soft-copy of the stable release changelog. Leave blank or remove the lines if no entry in the stable changelog is needed. Entries should be on the same line, without the `{{` `}}` brackets. You can use multiple lines, even of the same type. The valid suffixes are: * NEW-FEATURE: for new, relatively sizable features. Features listed here will likely have docs / social media posts / marketing launches associated with them, so use sparingly. * IMPROVEMENT: for new functionality of existing features. * BUG-FIX: for fixes related to known bugs or regressions. * IMAGE: the image specified by the URL (hosted on GCP) will be added to Dev & Preview releases. For Stable releases, see the pinned doc in the #release Slack channel. * OZ: Oz-related updates. Use `CHANGELOG-OZ`. At most 4 Oz updates are shown in-app per release. CHANGELOG-NEW-FEATURE: {{text goes here...}} CHANGELOG-IMPROVEMENT: {{text goes here...}} CHANGELOG-BUG-FIX: {{text goes here...}} CHANGELOG-BUG-FIX: {{more text goes here...}} CHANGELOG-IMAGE: {{GCP-hosted URL goes here...}} CHANGELOG-OZ: {{text goes here...}} -->
) 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 ## Description <!-- Please remember to add your design buddy onto the PR for review, if it contains any UI changes! --> What is a design buddy? ## Linked Issue <!-- Link the GitHub issue this PR addresses. Before opening this PR, please confirm: --> - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [X] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Screenshots / Videos <img width="1774" height="1326" alt="Screenshot_redacted_2" src="https://github.com/user-attachments/assets/596bb770-d64f-443c-9da9-30e4fe0bf727" /> ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? --> Look at the PR. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Used claude code. <!-- ## Changelog Entries for Stable The entries below will be used when constructing a soft-copy of the stable release changelog. Leave blank or remove the lines if no entry in the stable changelog is needed. Entries should be on the same line, without the `{{` `}}` brackets. You can use multiple lines, even of the same type. The valid suffixes are: * NEW-FEATURE: for new, relatively sizable features. Features listed here will likely have docs / social media posts / marketing launches associated with them, so use sparingly. * IMPROVEMENT: for new functionality of existing features. * BUG-FIX: for fixes related to known bugs or regressions. * IMAGE: the image specified by the URL (hosted on GCP) will be added to Dev & Preview releases. For Stable releases, see the pinned doc in the #release Slack channel. * OZ: Oz-related updates. Use `CHANGELOG-OZ`. At most 4 Oz updates are shown in-app per release. CHANGELOG-NEW-FEATURE: {{text goes here...}} CHANGELOG-IMPROVEMENT: {{text goes here...}} CHANGELOG-BUG-FIX: {{text goes here...}} CHANGELOG-BUG-FIX: {{more text goes here...}} CHANGELOG-IMAGE: {{GCP-hosted URL goes here...}} CHANGELOG-OZ: {{text goes here...}} --> (cherry picked from commit 1d1c06d)
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.