Skip to content

fix: use actual terminal height for scroll instead of hardcoded value#47

Merged
abhixdd merged 2 commits intoabhixdd:mainfrom
LuisMIguelFurlanettoSousa:fix/hardcoded-visible-height
Apr 2, 2026
Merged

fix: use actual terminal height for scroll instead of hardcoded value#47
abhixdd merged 2 commits intoabhixdd:mainfrom
LuisMIguelFurlanettoSousa:fix/hardcoded-visible-height

Conversation

@LuisMIguelFurlanettoSousa
Copy link
Copy Markdown
Contributor

Summary

Fixes #46

The scroll calculation now uses the actual terminal height instead of a hardcoded visible_height = 10.

Changes

  • Added terminal_height: u16 field to AppState (default: 24)
  • Updated event loop to set terminal_height from f.size().height each frame
  • adjust_scroll now calculates visible height by subtracting UI chrome (breadcrumb, borders, header row, help bar ≈ 8 lines) from the actual terminal height

Before

All terminals → scroll window fixed at 10 lines

After

Terminal with 24 rows → ~16 visible lines
Terminal with 50 rows → ~42 visible lines
Terminal with 12 rows → ~4 visible lines (gracefully handles small terminals)

O adjust_scroll usava visible_height = 10 hardcoded, o que
causava problemas em terminais de diferentes tamanhos:

- Terminais pequenos: cursor podia ficar fora da área visível
- Terminais grandes: scroll ficava restrito a janela de 10
  linhas mesmo com espaço disponível

Agora terminal_height é atualizado a cada frame via f.size()
e adjust_scroll calcula a altura visível descontando o
chrome da UI (breadcrumb, bordas, header, help bar).
@abhixdd
Copy link
Copy Markdown
Owner

abhixdd commented Mar 27, 2026

@LuisMIguelFurlanettoSousa Looks good overall, but one issue before merge: scroll height is still hardcoded, so when search/download bars show up, cursor and scroll can get out of sync. Please make adjust_scroll account for those extra rows (or use actual list height), then it’s good.

adjust_scroll now dynamically adds +2 for download status bar and +3
for search bar when they are visible, instead of using a fixed
chrome_height of 8. This prevents cursor/scroll desync when those
bars appear or disappear.
@LuisMIguelFurlanettoSousa
Copy link
Copy Markdown
Contributor Author

@abhixdd Fixed! adjust_scroll now dynamically accounts for the search and download bars:

let mut chrome_height: u16 = 8; // base: breadcrumb + borders + header + help
if self.downloading {
    chrome_height += 2; // download status bar
}
if self.is_searching {
    chrome_height += 3; // search bar with borders
}

This matches the exact Constraint::Length values used in browser.rs layout (download = Length(2), search = Length(3)), so scroll and cursor stay in sync regardless of which bars are visible.

@abhixdd
Copy link
Copy Markdown
Owner

abhixdd commented Apr 2, 2026

@LuisMIguelFurlanettoSousa Thanks for the fix. LGTM! Merging.

@abhixdd abhixdd merged commit 0f79639 into abhixdd:main Apr 2, 2026
3 checks passed
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.

Bug: Scroll uses hardcoded visible_height=10, breaks on different terminal sizes

2 participants