[NRT-790] Fix clipped last line of long tag checkboxes in suggestion dialog#1309
Open
RisingOrange wants to merge 6 commits into
Open
[NRT-790] Fix clipped last line of long tag checkboxes in suggestion dialog#1309RisingOrange wants to merge 6 commits into
RisingOrange wants to merge 6 commits into
Conversation
…dialog _WrappingCheckBox.heightForWidth measured wrapping against a wider width than paintEvent drew into: the style content-rect metrics were cached in __init__ before the widget was polished, and the dialog's stylesheet pads the checkbox content rect once QStyleSheetStyle is active. The cached value was too wide, so heightForWidth under-counted wrapped lines and reserved one line too few, clipping the last segment of deep tags. boundingRect was also a different wrapping engine than the painter, disagreeing at wrap boundaries. - Compute the label content width fresh per call (same SE_CheckBoxContents the painter uses) instead of caching a pre-polish value. - Share one QTextLayout between heightForWidth and paintEvent so reserved height matches the painted line count exactly. - Wrap at word boundary or anywhere so an over-long unbreakable leaf wraps instead of overflowing horizontally. Adds a dialog-level regression test covering both vertical (deep tag) and horizontal (long leaf) clipping in the real dialog.
- Measure painted extents through production _lay_out/_content_width instead of re-implementing the QTextLayout loop, so the test asserts the real paint output fits rather than a copy of it. - Restore the app font in a finally block so the larger-font setup doesn't leak into other tests in the process. - Wait for layout to settle via qtbot.waitUntil instead of a fixed 10ms timer. - Account for the paint top offset in the vertical-clip assertion. - Drop now-redundant inline imports (SuggestionDialog, aqt.qt).
…ped helper The tag-box dicts are typed Dict[str, QCheckBox], so calling _vertical_padding() on a loop var tripped mypy. Compute the text top offset inside painted_extents (where cb is untyped) and return it.
The bug is a pre-polish vs post-polish discrepancy in the checkbox's content-rect metrics, which reproduces on a bare _WrappingCheckBox once it is shown — it does not need the full SuggestionDialog or an Anki collection. Replace the heavy profile_loaded() integration test with a fast widget test that shows the checkbox (to polish it) and asserts heightForWidth reserves enough height for the painted text across widths and font sizes. Verified it fails on the cached-pre-polish bug and passes on the fix.
ddevdan
reviewed
Jun 9, 2026
ddevdan
left a comment
ddevdan
left a comment
Contributor
There was a problem hiding this comment.
LGTM but we need to create the addon for Maryna/Daemon to do QA
Drop the 360px cap on the Include-in-suggestion panel so the existing 2:3 stretch factors keep both columns growing with the window. The cap predates the elision/wrapping work; long titles and tags can no longer push the dialog wide.
ddevdan
approved these changes
Jun 10, 2026
ddevdan
left a comment
ddevdan
left a comment
Contributor
There was a problem hiding this comment.
LGTM
But, just a heads up
Next steps before merging:
- Create PR env
- Set the feature flag there
- Create addon pointing to the PR env
- Move to Ready for QA
Contributor
|
I was wondering if we could fix this, but I think it's more of a bug on Anki's side than in the add-on. 
|
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
2 participants
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.
Related issues
Proposed changes
1. Fix clipped last line of long tag checkboxes
In the note suggestion dialog ("Include in suggestion" panel, behind
auto_protect_fields_when_edited), long tag checkboxes had their last line clipped:#AK_MCAT_v2::MileDown::Behavioral::Cognitionlost its bottom segment ("Cognition") — the row wrapped to one more line than its reserved height. The clip appeared/disappeared at boundary widths when resizing the dialog.More noticeable with larger UI fonts.
Root cause:
_WrappingCheckBox.heightForWidthmeasured text wrapping against a wider width thanpaintEventactually drew into. The style content-rect metrics were cached in__init__, before the widget was polished — and the dialog's stylesheet activatesQStyleSheetStyle, which pads the checkbox content rect once polished, so the cached width was ~6px too wide andheightForWidthreserved one line too few. Wrapping was also measured withQFontMetrics.boundingRect, a different code path than the painter, which disagrees at wrap boundaries. This is why it only reproduced in the real dialog, not in an isolated widget.Fix:
SE_CheckBoxContentsthe painter uses) instead of caching a pre-polish value.QTextLayoutbetweenheightForWidthandpaintEventso the reserved height matches the painted line count exactly.2. Scale both columns proportionally when resizing (review feedback)
Previously, enlarging the dialog gave almost all extra width to the right (Change Type) column: the "Include in suggestion" panel was capped at 360px, even though it's the column that benefits most from extra room (long tags). The columns already had 2:3 stretch factors — the cap just kept them from applying. Dropping the cap lets both columns grow proportionally (~40/60). Long content still can't push the dialog wide: section titles and tag rows elide (
Ignoredsize policy) and the rest sits inside a scroll area.How to reproduce
auto_protect_fields_when_editedfeature flag.#AK_MCAT_v2::MileDown::Behavioral::Cognition(and/or add a tag with a very long final segment).Screenshots and videos
Suggestion dialog with a deep tag and a long-leaf tag, before and after the clipping fix:
Proportional column scaling at a wide window size (1500px):
Further comments
TestFieldsToSuggestFilters::test_long_tag_checkbox_is_not_clipped, a dialog-level regression test that reproduces both the vertical (deep tag) and horizontal (long leaf) clipping in the real dialog and asserts no clipping across a range of widths. Verified it fails on the buggy version and passes on the fix.mypyclean.QTextLayoutbuild replace a cache that was the source of the bug; reviewers (Codex + cleanup pass) confirmed the cost is negligible at realistic tag counts, so no caching was reintroduced.