fix(viewing): replace font metrics with document renderer #422
Conversation
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
Upgrade the height computation mechanism to utilize QTextDocument instead of QFontMetrics. This change accommodates HTML formatting in the license display and adjusts minimum dimensions to improve visual proportions across different content volumes. log: replace font metrics with document renderer pms: BUG-314073
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces font-metrics-based height calculation with a QTextDocument-based HTML-aware content height computation and adjusts scroll area minimum height bounds to better fit varying content lengths. Sequence diagram for updated window height calculationsequenceDiagram
actor User
participant LicenseDialog
participant Content
participant QTextDocument
participant QScrollArea
User->>LicenseDialog: open()
LicenseDialog->>Content: updateWindowHeight()
Content->>QTextDocument: construct()
Content->>QTextDocument: setHtml(m_source.text())
Content->>QTextDocument: setTextWidth(m_scrollArea.width() - 40)
QTextDocument-->>Content: size().height()
Content->>QScrollArea: setMinimumHeight(qBound(100, contentHeight, 491))
Class diagram for Content height computation updateclassDiagram
class Content {
- QWidget m_source
- QScrollArea m_scrollArea
+ updateWindowHeight()
}
class QTextDocument {
+ setHtml(html)
+ setTextWidth(width)
+ size()
}
Content ..> QTextDocument : uses
Content ..> QWidget : reads_text
Content ..> QScrollArea : sets_minimumHeight
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码修改主要是为了优化 1. 语法逻辑审查现状：
潜在问题：
2. 代码质量审查优点：
改进建议：
3. 代码性能审查潜在问题：
改进建议：
4. 代码安全审查潜在问题：
改进建议：
5. 其他改进建议
优化后的代码示例void Content::updateWindowHeight()
{
static QString lastText;
static int lastWidth = 0;
static int cachedHeight = 0;
QString currentText = m_source->text();
const int kTextMargin = 40;
int availableWidth = m_scrollArea->width() - kTextMargin;
if (availableWidth <= 0) {
availableWidth = 100; // 默认宽度
}
if (currentText != lastText || availableWidth != lastWidth) {
QTextDocument doc;
doc.setHtml(currentText); // 或 setPlainText，根据实际需求
doc.setTextWidth(availableWidth);
cachedHeight = static_cast<int>(doc.size().height());
lastText = currentText;
lastWidth = availableWidth;
}
int minHeight = qBound(100, cachedHeight, 491);
m_scrollArea->setMinimumHeight(minHeight);
}总结新代码在逻辑上是正确的，但可以通过以下方式进一步优化：
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Using
m_scrollArea->width()as the layout width may be fragile if this is called before the scroll area is laid out (width could be 0 or stale); consider basing it onm_source’s available width or ensuring this is only invoked after layout is finalized or on resize. - The bounds for
minHeighthave changed from[300, 491]to[100, 491], which can significantly reduce the dialog’s minimum height; double-check whether this is intentional for all license contents or whether a higher lower-bound (or content-dependent heuristic) would keep the layout more consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `m_scrollArea->width()` as the layout width may be fragile if this is called before the scroll area is laid out (width could be 0 or stale); consider basing it on `m_source`’s available width or ensuring this is only invoked after layout is finalized or on resize.
- The bounds for `minHeight` have changed from `[300, 491]` to `[100, 491]`, which can significantly reduce the dialog’s minimum height; double-check whether this is intentional for all license contents or whether a higher lower-bound (or content-dependent heuristic) would keep the layout more consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
caixr23
approved these changes
Jan 30, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, caixr23 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Upgrade the height computation mechanism to utilize QTextDocument instead of QFontMetrics. This change accommodates HTML formatting in the license display and adjusts minimum dimensions to improve visual proportions across different content volumes.
log: replace font metrics with document renderer
pms: BUG-314073
Summary by Sourcery
Update license dialog content height calculation to use rich-text document rendering instead of font metrics for better handling of HTML content and proportional sizing.
Bug Fixes:
Enhancements: