Skip to content

editor: Fix completion menu intrinsic width measurement - #2571

Merged
huacnlee merged 2 commits into
longbridge:mainfrom
wabzqem:fix/completion-menu-width
Jul 30, 2026
Merged

editor: Fix completion menu intrinsic width measurement#2571
huacnlee merged 2 commits into
longbridge:mainfrom
wabzqem:fix/completion-menu-width

Conversation

@wabzqem

@wabzqem wabzqem commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Completion menus already select their longest label/detail row with ListState::set_item_to_measure_index, but that representative row was not reaching VirtualList's cross-axis measurement. VirtualList always measured flattened item 0, which is commonly an empty section-header entry, so the popover collapsed to its 120px minimum.

This change passes the selected row's flattened index through ListState to VirtualList and uses that item when inferring content width. The existing completion popover constraints remain unchanged: short lists stay compact, longer lists expand to content, and width remains capped by the existing 320px/available-space maximum.

Screenshot

Before After
Screenshot 2026-07-15 at 5 12 26 pm Screenshot 2026-07-15 at 5 14 10 pm

How to Test

  1. Use an LSP server such as rust-analyzer and trigger a completion list with short items; confirm the popup remains compact.
  2. Trigger a list containing longer labels/details; confirm the popup expands beyond 120px.
  3. Confirm very long items remain capped at the existing maximum and documentation stays adjacent to the actual completion width.
  4. Run cargo check -p gpui-component and cargo test -p gpui-component.

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
  • Passed cargo run for story tests related to the changes.

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change is definitely wrong; it's not the right way to solve the problem.

@wabzqem
wabzqem force-pushed the fix/completion-menu-width branch from 277395e to 8c948b0 Compare July 15, 2026 09:34
@wabzqem wabzqem changed the title Fix completion menu width collapsing to minimum Fix completion menu intrinsic width measurement Jul 15, 2026
@wabzqem

wabzqem commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Apologies, I should have looked deeper than that.

I've pushed a (hopefully better) fix that keeps the existing min/max popup constraints and passes the selected flattened row index through to VirtualList intrinsic measurement. Short lists remain compact, long lists expand to content, and very long lists remain capped. I validated with the full gpui-component test suite (313 tests).

@wabzqem
wabzqem requested a review from huacnlee July 15, 2026 22:13
@huacnlee huacnlee changed the title Fix completion menu intrinsic width measurement editor: Fix completion menu intrinsic width measurement Jul 30, 2026
Co-authored-by: Codex <codex@openai.com>
@huacnlee
huacnlee enabled auto-merge (squash) July 30, 2026 12:42
@huacnlee
huacnlee merged commit 88f102d into longbridge:main Jul 30, 2026
3 checks passed
dwander added a commit to dwander/gpui-component that referenced this pull request Aug 10, 2026
충돌: crates/ui/src/virtual_list.rs

업스트림(longbridge#2571)이 교차축 content_size 대입을 sizes_changed 분기 밖으로 옮겼다.
업스트림은 매 레이아웃 패스마다 측정하므로 맞는 코드지만, 우리 포크는 item_sizes
변경 시에만 측정한다(0f89a73). 그대로 병합하면 측정을 건너뛴 모든 패스에서
longest_item_size 가 0이라 그리드 폭이 0으로 덮어써진다 — 컴파일은 통과하므로
빌드만으로는 드러나지 않는다. 썸네일 그리드(v_virtual_list)와 필름스트립
(h_virtual_list)이 바로 이 경로를 쓴다.

해결:
- 측정을 건너뛰면 캐시된 교차축 크기를 건드리지 않도록 Option 으로 감쌌다.
- 업스트림 신규 with_item_to_measure_index(자동완성 메뉴 폭 수정용, list.rs 에서
  실제 사용 중)를 존중하도록 ItemSizeLayout 에 measured_item_ix 를 추가해
  측정 인덱스가 바뀌면 재측정한다.
- measure_item 은 인라인 유지 — &self 헬퍼는 self.base 가변 대여와 충돌한다(E0502).

앱 실행으로 그리드 렌더 폭 정상 확인. 포크 패치 8개는 모두 온전하다.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants