list: measure an existing row when the configured item is absent - #7
Closed
grishy wants to merge 3 commits into
Closed
list: measure an existing row when the configured item is absent#7grishy wants to merge 3 commits into
grishy wants to merge 3 commits into
Conversation
An absent measurement row produces a zero-height wrapper even when other sections contain items. Validate the configured index and fall back to a real row without overwriting the caller's preference.
3 tasks
Owner
Author
|
Opened upstream as longbridge#2975 with the reviewed patch and description. Closing this review PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
List measures one row to determine the height of its items. When the configured row is missing, it measures an empty element at zero height. A remaining row can then overlap the section footer.
The measurement row is chosen in this order:
The fallback is only used for the current measurement. It does not overwrite
item_to_measure_index, which is the caller's setting. For example, if row 5 is configured but filtering leaves only two rows, the list measures row 0 without changing the setting to 0. Once row 5 is a valid index again, the list measures it. This preserves the configured position, not the identity of a particular item.The choice stays in
prepare_items_if_needed, where List already measures its rows. Selection, filtering, and header/footer measurement are unchanged.Screenshots
Both captures use the same fixed-data Story setup, with an empty first section and search results replaced on each query. Only the measurement fix differs. This setup is not included in the PR.
How to Test
The regression reproduces the problem without any Story changes:
cargo test -p gpui-component --lib measures_an_existing_row_when_the_requested_item_is_absent --lockedWithout the fix, the test fails with
0pxinstead of36px. It covers missing rows and sections, an empty list, and a configured index becoming invalid and then valid again. Different row heights verify that measurement returns to the configured index without resetting it.I also manually checked filtering, empty results, clearing the query, repeated searches, and window resizing on macOS. Rows and the section footer stayed separated.
Automated checks
cargo test -p gpui-base -p gpui-component --lib --locked cargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets --locked -- -D warnings771 base and 419 component tests passed, along with strict Clippy, formatting, and the diff whitespace check.
Windows and Linux runtime were not tested.
Checklist
AI assistance
I tested this fix in my application, then asked OpenAI's
gpt-6-astrato extract it into a standalone PR. The agent prepared the patch, regression test, and description, and ran the automated checks. I reviewed the extracted patch, requested corrections, and manually tested it again on macOS.