Skip to content

fix: dateTime plugin only responds to short format changes from dde-c…#320

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix-shortTimeFromDcc
Jun 9, 2025
Merged

fix: dateTime plugin only responds to short format changes from dde-c…#320
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix-shortTimeFromDcc

Conversation

@yixinshark

@yixinshark yixinshark commented Jun 6, 2025

Copy link
Copy Markdown
Contributor

…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:

  • Hide the nonfunctional 12/24-hour toggle menu item in the datetime plugin

Enhancements:

  • Always use the region’s short time format for rendering time displays
  • Strip ‘AP’ markers and extra spaces when displaying AM/PM times

@sourcery-ai

sourcery-ai Bot commented Jun 6, 2025

Copy link
Copy Markdown

Reviewer's Guide

Refactors 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 Components

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

File-Level Changes

Change Details Files
Unify time format handling in DatetimeWidget
  • Always retrieve the short time format from m_regionFormat
  • Replace legacy hh:mm and h:mm literals with region format
  • Strip ‘AP’ and whitespace when rendering 12-hour time
plugins/dde-dock/datetime/datetimewidget.cpp
Hide 12/24-hour toggle in context menu
  • Wrap time-setting menu entries in #if 0/#endif
  • Disable insertion of 12-hour and 24-hour items
plugins/dde-dock/datetime/datetimeplugin.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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 on lines 180 to +183
settings["itemText"] = tr("24-hour time");
settings["isActive"] = true;
items.push_back(settings);

#endif

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Suggested change
settings["itemText"] = tr("24-hour time");
settings["isActive"] = true;
items.push_back(settings);
#endif

@deepin-ci-robot

Copy link
Copy Markdown

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

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

…ontrol-center

1.datetime plugin has no time format menu
2.only responds to short format changes from dcc.

Pms: BUG-303071,BUG-286253
@yixinshark yixinshark force-pushed the fix-shortTimeFromDcc branch from 2dc9baf to 32aacee Compare June 9, 2025 02:16
@deepin-ci-robot

Copy link
Copy Markdown

@yixinshark: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-trigger-obs-ci 32aacee link true /test github-trigger-obs-ci

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查意见:

  1. datetimeplugin.cpp文件中,注释掉了时间设置项,但未移除相关代码。建议移除注释掉的代码,以保持代码的整洁性。

  2. datetimeplugin.cpp文件中,使用了#if 0来注释掉代码块,这是一种有效的做法,但建议在注释中说明为什么需要隐藏时间设置,以便其他开发者理解这一变更的背景。

  3. datetimewidget.cpp文件中,updateDateTimeString函数中对于24小时制和非24小时制的处理逻辑存在重复代码。建议提取公共代码到一个单独的函数中,以减少代码重复和提高代码的可维护性。

  4. datetimewidget.cpp文件中,对于非24小时制的时间格式处理,使用了replace方法来移除"AP"和空格,这种方法可能会导致时间格式不正确。建议使用更可靠的方法来处理时间格式,例如使用QDateTime::TimeSpec来指定时间格式。

  5. datetimewidget.cpp文件中,对于24小时制和非24小时制的处理逻辑,使用了不同的时间格式字符串。建议统一使用m_regionFormat->getShortTimeFormat()来获取时间格式,以避免潜在的格式不一致问题。

  6. datetimewidget.cpp文件中,对于非24小时制的时间格式处理,使用了replace方法来移除"AP"和空格。这种方法可能会导致时间格式不正确。建议使用更可靠的方法来处理时间格式,例如使用QDateTime::TimeSpec来指定时间格式。

  7. datetimewidget.cpp文件中,对于24小时制和非24小时制的处理逻辑,使用了不同的时间格式字符串。建议统一使用m_regionFormat->getShortTimeFormat()来获取时间格式,以避免潜在的格式不一致问题。

综上所述,建议对代码进行重构,以提高代码的可读性、可维护性和正确性。

@yixinshark

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jun 9, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 299b377 into linuxdeepin:master Jun 9, 2025
6 of 7 checks passed
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