fix(view): fix schedule label misalignment in week/day view#327
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts schedule reminder widget positioning in week/day view to use widget-relative coordinates and device pixel ratio–aware offsets, and removes obsolete screen-geometry code and includes. Sequence diagram for schedule reminder positioning with widget-relative coordinatessequenceDiagram
actor User
participant CalendarWindow
participant CScheduleView
participant ScheduleRemindWidget
User->>CalendarWindow: click schedule item
CalendarWindow->>CScheduleView: slotScheduleShow(isShow, out, pos22)
CScheduleView->>CScheduleView: setData(out, gdColor)
CScheduleView->>CScheduleView: ratio = devicePixelRatioF()
CScheduleView->>CScheduleView: rPos = mapFromGlobal(pos22)
CScheduleView->>CScheduleView: check rPos.x + width + 15 * ratio > window().width()
alt widget would overflow right edge
CScheduleView->>ScheduleRemindWidget: setDirection(ArrowRight)
CScheduleView->>ScheduleRemindWidget: show(rPos.x - 15 * ratio, rPos.y)
else fits on the right
CScheduleView->>ScheduleRemindWidget: setDirection(ArrowLeft)
CScheduleView->>ScheduleRemindWidget: show(rPos.x + 15 * ratio, rPos.y)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The offset
static_cast<int>(15 * ratio)is computed multiple times with the same value; consider calculating it once in a named local variable (e.g.,offsetPx) to reduce repetition and clarify its intent. - Now that you're using
mapFromGlobal(pos22)andthis->window()->width(), it would be helpful to double-check and document (via a short comment) which coordinate systempos22andrPosare in so future changes don’t mix global and widget-relative coordinates by accident.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The offset `static_cast<int>(15 * ratio)` is computed multiple times with the same value; consider calculating it once in a named local variable (e.g., `offsetPx`) to reduce repetition and clarify its intent.
- Now that you're using `mapFromGlobal(pos22)` and `this->window()->width()`, it would be helpful to double-check and document (via a short comment) which coordinate system `pos22` and `rPos` are in so future changes don’t mix global and widget-relative coordinates by accident.
## Individual Comments
### Comment 1
<location path="src/calendar-client/src/customWidget/scheduleview.cpp" line_range="540-549" />
<code_context>
+ qreal ratio = this->devicePixelRatioF();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `devicePixelRatioF()` for the 15px offset might be mixing logical and device pixels.
Previously the 15px margin was applied in the same logical coordinate space as `pos22` and `m_ScheduleRemindWidget->width()`. Now `rPos` and `window()->width()` remain in logical coordinates, but the 15px offset is being scaled by `devicePixelRatioF()`, so on high-DPI displays the visual margin will increase. If the rest of the layout is using Qt logical pixels (the usual case with high-DPI scaling), the offset should likely remain an unscaled logical value (15) rather than being multiplied by the DPR.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Convert global screen coordinates to widget-relative coordinates using mapFromGlobal() for proper positioning. Use logical pixel offset to avoid mixing coordinate systems. Remove unused QDesktopWidget include. 修复周/日视图下日程标签位置错位的问题,通过mapFromGlobal 将全局坐标转为控件相对坐标,使用逻辑像素偏移量。 Log: 修复周/日视图日程标签位置错位 PMS: BUG-356301 Influence: 修复后周/日视图下日程标签在窗口最大化及不同DPI下正确显示在日程旁边。
ebb033d to
422bbc4
Compare
deepin pr auto review这段代码的修改主要是为了移除对Qt 5的兼容性代码,并优化了窗口定位的逻辑。以下是我的审查意见: 1. 语法逻辑审查问题点:
改进建议: // 方案1:保持全局坐标系
const int offsetPx = 15;
if ((pos22.x() + m_ScheduleRemindWidget->width() + offsetPx) > this->window()->geometry().right()) {
// ...
m_ScheduleRemindWidget->show(pos22.x() - offsetPx, pos22.y());
} else {
// ...
m_ScheduleRemindWidget->show(pos22.x() + offsetPx, pos22.y());
}
// 方案2:如果确实需要使用相对坐标
const auto rPos = this->mapFromGlobal(pos22);
const int offsetPx = 15;
const int widgetRight = rPos.x() + m_ScheduleRemindWidget->width() + offsetPx;
const int parentWidth = this->width(); // 使用父控件宽度而不是窗口宽度
if (widgetRight > parentWidth) {
// ...
m_ScheduleRemindWidget->show(rPos.x() - offsetPx, rPos.y());
} else {
// ...
m_ScheduleRemindWidget->show(rPos.x() + offsetPx, rPos.y());
}2. 代码质量审查优点:
改进建议:
// 在类定义中添加
private:
static constexpr int WIDGET_OFFSET = 15;
static constexpr int MIN_WINDOW_WIDTH = 200;
// 使用时
if (this->window()->width() < MIN_WINDOW_WIDTH) {
qCWarning(ClientLogger) << "Window too small for proper widget positioning";
return;
}3. 代码性能审查当前代码性能良好,没有明显的性能问题:
4. 代码安全审查潜在问题:
改进建议: if (!m_ScheduleRemindWidget || !this->window()) {
qCWarning(ClientLogger) << "Invalid widget or window pointer";
return;
}
const auto rPos = this->mapFromGlobal(pos22);
const int offsetPx = 15;
// 添加坐标范围检查
if (rPos.x() < 0 || rPos.y() < 0) {
qCWarning(ClientLogger) << "Invalid coordinates:" << rPos;
return;
}
// 其余逻辑...5. 其他建议
总体来说,这段代码的修改方向是正确的,但需要完善边界检查和坐标系处理逻辑,确保在各种窗口布局下都能正确显示。 |
Convert global screen coordinates to widget-relative coordinates using mapFromGlobal() and account for device pixel ratio when calculating label offset. Remove unused QDesktopWidget include.
修复周/日视图下日程标签位置错位的问题,通过mapFromGlobal
将全局坐标转为控件相对坐标,并考虑屏幕缩放比计算偏移量。
Log: 修复周/日视图日程标签位置错位
PMS: BUG-356301
Influence: 修复后周/日视图下日程标签在窗口最大化及不同缩放比下正确显示在日程旁边。
Summary by Sourcery
Fix positioning of schedule reminder labels in week/day views to correctly align beside schedules across window sizes and display scaling.
Bug Fixes:
Enhancements: