fix(dialog): update my schedule icon theme sync and scroll behavior#333
Open
wangruoxuan3782 wants to merge 1 commit into
Open
fix(dialog): update my schedule icon theme sync and scroll behavior#333wangruoxuan3782 wants to merge 1 commit into
wangruoxuan3782 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wangruoxuan3782 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 |
|
Hi @wangruoxuan3782. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates the "My Schedule" dialog to make its icon follow the system theme and refines the scroll behaviour of the schedule content area so scrollbars and scrolling only appear when needed. Sequence diagram for theme-based dialog icon updatesequenceDiagram
participant DGuiApplicationHelper
participant CMyScheduleView
DGuiApplicationHelper->>CMyScheduleView: themeTypeChanged(type)
CMyScheduleView->>CMyScheduleView: setLabelTextColor(type)
CMyScheduleView->>CMyScheduleView: updateDialogIcon()
CMyScheduleView->>CMyScheduleView: setIcon(QIcon::fromTheme(dde-calendar))
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 found 1 issue, and left some high level feedback:
- The hard-coded height thresholds (
100and17) used to determine scrolling and minimum label height would be clearer and easier to maintain if extracted into named constants or configuration values. updateDialogIcon()switches fromCDynamicIcon::getInstance()->getPixmap()toQIcon::fromTheme("dde-calendar"); if theme-based behavior or fallback logic is encapsulated inCDynamicIcon, consider reusing it here to keep icon handling consistent across the app.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded height thresholds (`100` and `17`) used to determine scrolling and minimum label height would be clearer and easier to maintain if extracted into named constants or configuration values.
- `updateDialogIcon()` switches from `CDynamicIcon::getInstance()->getPixmap()` to `QIcon::fromTheme("dde-calendar")`; if theme-based behavior or fallback logic is encapsulated in `CDynamicIcon`, consider reusing it here to keep icon handling consistent across the app.
## Individual Comments
### Comment 1
<location path="src/calendar-client/src/dialog/myscheduleview.cpp" line_range="98-103" />
<code_context>
}
- if (strList.count() * h > 100) {
+ const int scheduleContentH = strList.count() * h;
+ const bool needScroll = scheduleContentH > 100;
+ if (needScroll) {
m_scheduleLabelH = 100;
} else {
int minH = 17;
- m_scheduleLabelH = strList.count() * h;
+ m_scheduleLabelH = scheduleContentH;
</code_context>
<issue_to_address>
**suggestion:** Consider extracting the hardcoded height values into named constants for maintainability.
Since `100` (scroll threshold) and `17` (minimum label height) are reused and encode layout assumptions, please extract them into clearly named constants (e.g. `kScheduleScrollThreshold`, `kScheduleMinHeight`) or class members to document intent and simplify future UI changes.
Suggested implementation:
```cpp
resultStr += str;
}
static constexpr int kScheduleScrollThreshold = 100;
static constexpr int kScheduleMinHeight = 17;
const int scheduleContentH = strList.count() * h;
const bool needScroll = scheduleContentH > kScheduleScrollThreshold;
if (needScroll) {
m_scheduleLabelH = kScheduleScrollThreshold;
} else {
int minH = kScheduleMinHeight;
m_scheduleLabelH = scheduleContentH;
m_scheduleLabelH = m_scheduleLabelH >= minH ? m_scheduleLabelH : minH;
}
//更新控件高度
area->setFixedHeight(m_scheduleLabelH);
area->setVerticalScrollBarPolicy(needScroll ? Qt::ScrollBarAsNeeded : Qt::ScrollBarAlwaysOff);
area->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
m_scheduleLabel->setText(resultStr);
m_scheduleLabel->setFixedHeight(needScroll ? scheduleContentH : m_scheduleLabelH);
m_timeLabelH = 26;
```
If these layout constants are used elsewhere in `MyScheduleView` or other files, consider moving `kScheduleScrollThreshold` and `kScheduleMinHeight` to class scope (e.g. as `static constexpr` members in the corresponding header) or to an anonymous namespace at the top of this `.cpp` file, and then use those shared definitions instead of function-local ones.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+98
to
103
| const int scheduleContentH = strList.count() * h; | ||
| const bool needScroll = scheduleContentH > 100; | ||
| if (needScroll) { | ||
| m_scheduleLabelH = 100; | ||
| } else { | ||
| int minH = 17; |
There was a problem hiding this comment.
suggestion: Consider extracting the hardcoded height values into named constants for maintainability.
Since 100 (scroll threshold) and 17 (minimum label height) are reused and encode layout assumptions, please extract them into clearly named constants (e.g. kScheduleScrollThreshold, kScheduleMinHeight) or class members to document intent and simplify future UI changes.
Suggested implementation:
resultStr += str;
}
static constexpr int kScheduleScrollThreshold = 100;
static constexpr int kScheduleMinHeight = 17;
const int scheduleContentH = strList.count() * h;
const bool needScroll = scheduleContentH > kScheduleScrollThreshold;
if (needScroll) {
m_scheduleLabelH = kScheduleScrollThreshold;
} else {
int minH = kScheduleMinHeight;
m_scheduleLabelH = scheduleContentH;
m_scheduleLabelH = m_scheduleLabelH >= minH ? m_scheduleLabelH : minH;
}
//更新控件高度
area->setFixedHeight(m_scheduleLabelH);
area->setVerticalScrollBarPolicy(needScroll ? Qt::ScrollBarAsNeeded : Qt::ScrollBarAlwaysOff);
area->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
m_scheduleLabel->setText(resultStr);
m_scheduleLabel->setFixedHeight(needScroll ? scheduleContentH : m_scheduleLabelH);
m_timeLabelH = 26;
If these layout constants are used elsewhere in MyScheduleView or other files, consider moving kScheduleScrollThreshold and kScheduleMinHeight to class scope (e.g. as static constexpr members in the corresponding header) or to an anonymous namespace at the top of this .cpp file, and then use those shared definitions instead of function-local ones.
Contributor
Author
|
/ok-to-test |
|
@wangruoxuan3782: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
wangruoxuan3782
changed the title
wangruoxuan3782
force-pushed
the
fix-myschedule-icon-scroll
branch
from
July 3, 2026 03:06
8f6cc2a to
e78b248
Compare
1. 在主题切换后同步刷新我的日程弹窗图标，确保左上角应用图标跟随系统主题变化。 2. 优化日程内容区域高度与滚动条显示逻辑，内容未超出可视区域时关闭滚动条和滚动效果。 3. 在内容超出显示区域时保留垂直滚动能力，避免完整内容被截断。 1. Refresh the My Schedule dialog icon when the theme changes so the top-left app icon stays in sync with the system theme. 2. Refine the schedule content height and scrollbar logic so scrolling stays disabled when the content fits in the visible area. 3. Keep vertical scrolling available only when the content exceeds the visible area to avoid truncating longer content. PMS: BUG-368849 PMS: BUG-368851 Log: - Updated the My Schedule dialog icon when the theme changes. - Disabled unnecessary scrolling when schedule content fits in the visible area.
wangruoxuan3782
force-pushed
the
fix-myschedule-icon-scroll
branch
from
July 3, 2026 07:38
e78b248 to
30f82e5
Compare
wangruoxuan3782
changed the title
Contributor
|
TAG Bot New tag: 6.5.41 |
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.
修改内容：
Change summary:
PMS: BUG-368849
PMS: BUG-368851