gui: the open list lays out its own rows, so opening it no longer parses the fonts again - #130
Conversation
…ses the fonts again The rows of an open list were widget.List's under a theme override that took out the room between them. Fyne 2.8.1 gives an override a new scope at construction, at CreateRenderer and at every Refresh, and parses the fonts again for every scope a new string is drawn in - every opening was a new override, and every letter a filter made bold a new string. rowView puts one ListRow at every position of the arrangement, one row apart (rowStack), in a container.Scroll, with no override. Keyboard scrolling follows widget.List's scrollTo with no gap between rows. RowShowing and DrawnRows answer the rows in sight. listRowHeight measures the text instead of building a label on every call (the same number). Measured in the real window, interleaved, three runs after and four before: - ten openings with a new letter in the filter: live heap +158 MB -> +1.5 MB, font sets 24 -> 2, process at the end 326 MB -> 100-115 MB; - the window busy per opening with a letter: 22-28 ms -> 6 ms. Five stored menu screens are pixel for pixel the same, their trees without the toolkit's list. One picture changed: the catalogue's filtered state came up 124 px down with the keyboard's row out of sight, and now comes up at its top, as typing puts a list on the screen. An emptied filter in a list with nothing chosen kept the rows it had narrowed to - on widget.List as well. StartOn now draws either way. New guards: TestTheRowTheKeyboardIsOnIsAlwaysInSight (written before the change, green on widget.List) and TestEmptyingTheFilterOfAListWithNothingChosenDrawsEveryRowAgain. TestNoScreenStandsInAThemeOverride now walks what the open list and the catalogue draw. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (14)
🧰 Additional context used📓 Path-based instructions (10)Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).⚙️ CodeRabbit configuration file Files:
Verify tests check real behavior and would fail if the implementation were broken.⚙️ CodeRabbit configuration file Files:
Performance is a known weak spot of these projects.⚙️ CodeRabbit configuration file Files:
Applies only to code that builds or styles a GUI.⚙️ CodeRabbit configuration file Files:
User-facing changelog.⚙️ CodeRabbit configuration file Files:
SECURITY, HIGH PRIORITY.⚙️ CodeRabbit configuration file Files:
Go code.⚙️ CodeRabbit configuration file Files:
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.⚙️ CodeRabbit configuration file Files:
All code in this repository is written by an AI coding agent (Claude Code).⚙️ CodeRabbit configuration file Files:
Source excerpt: **Words a user reads are English, with a flat hyphen and no semicolons.**📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
🔇 Additional comments (14)
📝 WalkthroughWalkthroughOpen format lists now use a custom scrollable row view instead of a toolkit list. Filtering, row visibility, and keyboard navigation use the new view. Tests and screen fixtures cover the updated list behavior. ChangesFormat-list rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Merge Risk: ⚪ Minimal · up to The format list now draws its rows with a lighter custom view. Scrolling, filtering and keyboard navigation keep their behavior, and the stored screens are unchanged. No outstanding issues were found, and the change looks ready to merge. 🚥 Pre-merge checks | ✅ 12 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (12 passed)
Full details: No Obvious Performance ProblemsExplanation The PR introduces a non-virtualized list. Resolution Keep viewport virtualization for Full details: No Resource LeaksExplanation The new Resolution Use a bounded visible-row pool, such as Fyne's list pooling model, instead of allocating one
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
1 participant
The last theme override in the window is gone: the open list of formats lays out its own rows instead of drawing
widget.Listunder one.Fyne 2.8.1 gives an override a new scope at construction, at
CreateRendererand at everyRefresh, and parses the fonts again for every scope a new string is drawn in. Every opening of a list was a new override, and every letter the filter makes bold was a new string.What changed
parts/rowview.go:rowViewputs oneListRowat every position of the arrangement, one row apart (rowStack), in acontainer.Scroll. No override.widget.List'sscrollTowith no gap between rows.RowShowingandDrawnRowsanswer the rows in sight.listRowHeightmeasures the text instead of building a label on every call. It is the same number.widget.List, andStartOnnow redraws either way.Measured
Real window,
guilag -phases walk,menufilter,menu, interleaved: three runs after the change, four before.Pictures
listItemare gone.pdf) out of sight. It now comes up at its top, which is what the real window shows after typing.Guards
TestTheRowTheKeyboardIsOnIsAlwaysInSight(new). It was written before the change and is green onwidget.List.TestEmptyingTheFilterOfAListWithNothingChosenDrawsEveryRowAgain(new). It is red onmaintoo.TestNoScreenStandsInAThemeOverridenow walks what the open list and the catalogue draw.Also checked in the real window at 1.5 scale: open, wheel, hover, filter, arrows and Esc.
🤖 Generated with Claude Code
Summary by CodeRabbit