fix: dateTime plugin only responds to short format changes from dde-c…#320
Conversation
Reviewer's GuideRefactors datetime display to consistently use region-defined short time formats and removes the time format toggle from the context menu. Class Diagram: Updates to Datetime ComponentsclassDiagram
class DatetimeWidget {
-m_regionFormat: RegionFormat*
-m_24HourFormat: bool
+updateDateTimeString() void
}
class DatetimePlugin {
-m_centralWidget: DatetimeWidget*
+itemContextMenu(const QString &itemKey) const QString
}
class RegionFormat {
+getShortTimeFormat() QString
}
DatetimeWidget ..> RegionFormat : uses
DatetimePlugin o-- DatetimeWidget : aggregates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @yixinshark - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| settings["itemText"] = tr("24-hour time"); | ||
| settings["isActive"] = true; | ||
| items.push_back(settings); | ||
|
|
||
| #endif |
There was a problem hiding this comment.
suggestion: Avoid using #if 0 to hide code; prefer code removal or runtime flags
Preprocessor guards create dead code and hinder maintainability. Use a runtime flag or remove the code to keep the codebase clean.
| settings["itemText"] = tr("24-hour time"); | |
| settings["isActive"] = true; | |
| items.push_back(settings); | |
| #endif |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, yixinshark 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 |
…ontrol-center 1.datetime plugin has no time format menu 2.only responds to short format changes from dcc. Pms: BUG-303071,BUG-286253
2dc9baf to
32aacee
Compare
|
@yixinshark: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
deepin pr auto review代码审查意见:
综上所述,建议对代码进行重构,以提高代码的可读性、可维护性和正确性。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
…ontrol-center
1.datetime plugin has no time format menu
2.only responds to short format changes from dcc.
Pms: BUG-303071,BUG-286253
Summary by Sourcery
Enforce region-based short time formatting in the datetime widget and remove the manual 12/24-hour toggle from the context menu.
Bug Fixes:
Enhancements: