Skip to content

fix(view): fix schedule label misalignment in week/day view#327

Merged
pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/schedule-label-position
Apr 30, 2026
Merged

fix(view): fix schedule label misalignment in week/day view#327
pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/schedule-label-position

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

@pengfeixx pengfeixx commented Apr 30, 2026

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:

  • Correct schedule label alignment in week/day views by basing popup positioning on widget-relative coordinates and display scaling.

Enhancements:

  • Simplify screen handling by removing legacy QDesktopWidget usage and relying on widget and window geometry.

@deepin-ci-robot
Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 30, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts 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 coordinates

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Fix schedule reminder widget horizontal positioning by using widget-relative coordinates and scaling the offset by device pixel ratio instead of relying on global screen geometry.
  • Remove legacy screen-geometry retrieval using QDesktopWidget/QScreen and associated preprocessor conditionals.
  • Use mapFromGlobal to convert the global click/hover position to CScheduleView-relative coordinates for positioning the reminder widget.
  • Introduce a devicePixelRatioF-based scaling factor and apply it to the 15px horizontal offset used when placing the reminder widget left or right of the schedule.
  • Change boundary check to compare against the current window width instead of the primary screen width.
  • Update calls to m_ScheduleRemindWidget->show(...) to use the converted widget-relative coordinates with scaled offsets.
src/calendar-client/src/customWidget/scheduleview.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/calendar-client/src/customWidget/scheduleview.cpp Outdated
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下正确显示在日程旁边。
@pengfeixx pengfeixx force-pushed the fix/schedule-label-position branch from ebb033d to 422bbc4 Compare April 30, 2026 09:04
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是为了移除对Qt 5的兼容性代码,并优化了窗口定位的逻辑。以下是我的审查意见:

1. 语法逻辑审查

问题点:

  • 坐标系转换可能存在问题:代码将全局坐标 pos22 转换为控件相对坐标 rPos,然后使用 this->window()->width() 来判断边界。这里存在逻辑不一致:
    • rPos 是相对于 this(CScheduleView)的坐标
    • this->window()->width() 是整个窗口的宽度
    • 如果 this 不是窗口的根控件,这个判断可能不准确

改进建议:

// 方案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. 代码质量审查

优点:

  • 移除了Qt版本兼容代码,使代码更简洁
  • 添加了有意义的注释说明坐标转换
  • 使用了命名常量 offsetPx 提高可读性

改进建议:

  1. 建议将 offsetPx 提取为类成员常量,便于统一修改
  2. 可以添加边界检查,确保 m_ScheduleRemindWidget 不为空
  3. 考虑添加对窗口最小宽度的检查,避免在极小窗口下布局错乱
// 在类定义中添加
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. 代码性能审查

当前代码性能良好,没有明显的性能问题:

  • 移除了Qt版本检查的运行时判断
  • 坐标转换操作简单高效
  • 没有不必要的计算或内存分配

4. 代码安全审查

潜在问题:

  1. 空指针风险:没有检查 m_ScheduleRemindWidget 是否为空
  2. 窗口边界风险:没有检查 this->window() 是否为空
  3. 坐标越界风险:没有处理负坐标的情况

改进建议:

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. 其他建议

  1. 日志改进:可以添加更详细的调试信息,包括实际计算的坐标值

    qCDebug(ClientLogger) << "Positioning widget - rPos:" << rPos 
                          << "widgetWidth:" << m_ScheduleRemindWidget->width()
                          << "windowWidth:" << this->window()->width();
  2. 多屏支持:原代码移除了多屏支持,如果需要考虑多屏环境,建议保留相关逻辑

  3. DPI缩放:考虑高DPI环境下的显示效果,可能需要使用设备像素比进行调整

总体来说,这段代码的修改方向是正确的,但需要完善边界检查和坐标系处理逻辑,确保在各种窗口布局下都能正确显示。

@pengfeixx pengfeixx merged commit a503bc0 into linuxdeepin:master Apr 30, 2026
20 of 22 checks passed
@pengfeixx pengfeixx deleted the fix/schedule-label-position branch April 30, 2026 09:21
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.

3 participants