Skip to content

feat(gpui): list padding on all sides, averaged scroll extent, hover exit, font lookup fix - #10

Merged
remorses merged 5 commits into
remorses:gpuixfrom
rubnogueira:feat/list-padding-and-scroll-extent
Oct 11, 2026
Merged

remorses merged 5 commits into
remorses:gpuixfrom
rubnogueira:feat/list-padding-and-scroll-extent

Conversation

@rubnogueira

@rubnogueira rubnogueira commented Oct 11, 2026 •

Copy link
Copy Markdown

Summary

Four small gpui changes that GPUIX needs for its native scrollbars, hover handling and font loading. Each is independent and touches one file.

  • fix(gpui): forget failed font lookups when fonts are added (text_system.rs): a family that failed to resolve before a font was added stayed failed. Text that asked for it earlier now picks the font up on the next frame.
  • fix(gpui): hover nothing once the pointer leaves the window (div.rs, window.rs): the last hovered element kept its hover state, cursor and tooltip until the pointer came back to the window.
  • feat(gpui): list reports a scroll extent that averages unmeasured rows (list.rs): adds ListState::averaged_scroll_extent(). A scrollbar built from a list's own totals counts unmeasured rows as the size hint, which is several times off for a list of tall rows with a short hint. This counts them as the average measured row.
  • feat(gpui): a list lays its rows out inside its left and right padding (list.rs): the list honored top and bottom padding and ignored left and right, so rows spanned its full width. Rows now start at the left padding and get the width left between the two paddings. A change in either padding re-measures rows the way a change in the list's width does. Zero padding behaves as before.
  • docs(gpui): describe averaged_scroll_extent for any list of tall rows

Notes

  • Based on the tip of gpuix (ea042f2), so this is a fast-forward of five commits.
  • The remorses/gpuix submodule pointer moves to the head of this branch in the companion PR, remorses/gpuix#88. Merge this one first.

Test plan

  • Exercised through GPUIX's native addon: its react suite (536 tests) passes against an addon built from this branch, including pixel tests that check rows inset by left, right and top padding and a thumb at the list's edge.
  • gpui's own cargo test -p gpui list tests were not run on these commits.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T13:31:00.181916Z c2e9f88 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

self.font_ids_by_font
.write()
.retain(|_, font_id| font_id.is_ok());

P1 Badge Invalidate CosmicTextSystem's failed family cache

When a family is first requested on Linux before registration, CosmicTextSystem::font_id stores the empty result from load_family in font_ids_by_family_cache (crates/gpui_wgpu/src/cosmic_text_system.rs lines 145-151), while its add_fonts at lines 277-282 leaves that cache intact. This only removes the outer TextSystem error, so the next lookup immediately reuses the empty platform entry and fails again; dynamically added families still never resolve on the WGPU/Linux backend. Invalidate the platform family cache, or avoid caching empty candidates, when fonts are added.


zed/crates/gpui/src/window.rs

Lines 5366 to 5367 in c2e9f88

PlatformInput::MouseExited(_) => {
self.pointer_outside = true;

P1 Badge Synchronize pointer-outside with hover-status callbacks

On Windows, the checked WM_MOUSELEAVE path in crates/gpui_windows/src/events.rs::handle_mouse_leave_msg (lines 398-411) only updates hover status and invokes on_hover_status_change; it never emits PlatformInput::MouseExited. Because pointer_outside becomes true only in this arm, the hover callback's refresh re-hit-tests the last in-window position at window.rs lines 3236-3240, leaving hover styles and tooltips stuck as before. Update this state from the platform hover-status callback as well, and clear it on re-entry.


let total = summary.measured_height + average * summary.unrendered_count as f32;
let viewport = state.last_layout_bounds.unwrap_or_default().size.height;
let max = (total - viewport).max(px(0.));

P2 Badge Include vertical padding in the averaged scroll maximum

For a list with nonzero top or bottom padding, the actual scroll limit includes both edges (StateInner::scroll at lines 957-962 and is_scrolled_to_end at lines 500-507), but total here contains only row heights. The returned maximum is therefore short by the padding sum, and the bottom-aligned branch also reports that too-small value as its offset, causing a scrollbar thumb to reach its end before the list's real scroll range does.


Some((max, offset.min(max)))

P2 Badge Clamp averaged scroll offsets at zero

ListState::scroll_to explicitly accepts a negative offset_in_item for anchored splices, and until the following layout resolves it at lines 1104-1124, averaged_scroll_extent can observe that negative value. Using only min(max) leaves the returned offset negative despite this method's promise that both values are nonnegative, so a scrollbar queried during that frame receives an invalid position; clamp the lower bound as well.


// Rows are as wide as the list is inside its left and right padding.

P1 Badge Remove the redundant row-width comment

This comment only paraphrases the padded_width assignment immediately below rather than explaining a non-obvious reason, so it adds the kind of organizational/summary comment prohibited by the repository's Rust guidance; remove it and let the helper name carry the meaning.

AGENTS.md reference: AGENTS.md:L4-L4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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