refactor(gui): integrate debug dialog access via hidden click mechanism#716
Closed
pppanghu77 wants to merge 1 commit intolinuxdeepin:masterfrom
Closed
refactor(gui): integrate debug dialog access via hidden click mechanism#716pppanghu77 wants to merge 1 commit intolinuxdeepin:masterfrom
pppanghu77 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77 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 GuideAdds a reusable DebugDialog utility and wires it into the cooperation and data-transfer GUIs via a hidden 10-click mechanism on key labels, including timer-based click reset and log export/port-detection functionality, plus minor common utils and CMake updates. Sequence diagram for 10-click Basic Settings label debug dialog triggersequenceDiagram
actor User
participant SettingDialog
participant SettingDialogPrivate as D_ptr
participant basicLabel as BasicLabel
participant clickTimer as QTimer
participant DebugDialog
User->>basicLabel: Left mouse button release
basicLabel->>SettingDialog: eventFilter(watched, MouseButtonRelease)
SettingDialog->>D_ptr: handle basicLabel click
alt Left button clicked
D_ptr->>D_ptr: clickCount++
alt clickCount < CLICK_THRESHOLD
D_ptr->>clickTimer: start(3000)
else clickCount >= CLICK_THRESHOLD
D_ptr->>D_ptr: showDebugDialog()
D_ptr->>DebugDialog: create
DebugDialog->>DebugDialog: exec()
D_ptr->>D_ptr: resetClickCount()
end
SettingDialog-->>basicLabel: event handled (true)
else Other mouse button
SettingDialog-->>basicLabel: pass to base eventFilter
end
clickTimer-->>D_ptr: timeout()
D_ptr->>D_ptr: resetClickCount()
Sequence diagram for 10-click StartWidget title debug dialog triggersequenceDiagram
actor User
participant StartWidget
participant titleLabel as TitleLabel
participant clickTimer as QTimer
participant DebugDialog
User->>titleLabel: Left mouse button release
titleLabel->>StartWidget: eventFilter(obj, MouseButtonRelease)
alt obj == titleLabel and LeftButton
StartWidget->>StartWidget: clickCount++
alt clickCount < CLICK_THRESHOLD
StartWidget->>clickTimer: start(3000)
else clickCount >= CLICK_THRESHOLD
StartWidget->>StartWidget: showDebugDialog()
StartWidget->>DebugDialog: create
DebugDialog->>DebugDialog: exec()
StartWidget->>StartWidget: resetClickCount()
end
StartWidget-->>titleLabel: event handled (true)
else other object or button
StartWidget-->>titleLabel: pass to QFrame::eventFilter
end
clickTimer-->>StartWidget: timeout()
StartWidget->>StartWidget: resetClickCount()
Class diagram for DebugDialog and hidden click trigger integrationclassDiagram
class StartWidget {
+StartWidget(parent: QWidget*)
+void nextPage()
+void themeChanged(theme: int)
+bool eventFilter(obj: QObject*, event: QEvent*)
-void initUI()
-void resetClickCount()
-void showDebugDialog()
-QPushButton* nextButton
-QLabel* titleLabel
-QTimer* clickTimer
-int clickCount
-static int CLICK_THRESHOLD
}
class SettingDialog {
+SettingDialog(QWidget* parent)
+~SettingDialog()
+bool eventFilter(watched: QObject*, event: QEvent*)
-SettingDialogPrivate* d
}
class SettingDialogPrivate {
+SettingDialogPrivate(SettingDialog* qq)
+~SettingDialogPrivate()
+void initWindow()
+void createBasicWidget()
+void createTransferWidget()
+void createClipboardShareWidget()
+bool checkNameValid()
+void resetClickCount()
+void showDebugDialog()
-void initFont()
-void initWindowContent()
-void initConnections()
-SettingDialog* q
-QVBoxLayout* mainLayout
-QScrollArea* scrollWidget
-QWidget* contentWidget
-QGridLayout* contentLayout
-CooperationLabel* basicLabel
-QComboBox* findCB
-QComboBox* connectCB
-QLineEdit* nameEdit
-CooperationSwitchButton* devShareSwitchBtn
-CooperationSwitchButton* clipShareSwitchBtn
-FileChooserEdit* chooserEdit
-QStringList findComboBoxInfo
-QList~QPair~QString,QString~~ connectComboBoxInfo
-QFont groupFont
-QFont tipFont
-QTimer* clickTimer
-int clickCount
-static int CLICK_THRESHOLD
}
class DebugDialog {
+DebugDialog(parent: QWidget*)
+~DebugDialog()
-void initUI()
-void loadCurrentLogLevel()
-void onLogLevelChanged(index: int)
-void onExportLog()
-void onDetectPorts()
-void onRefreshPorts()
-void detectPortsInternal(targetIP: QString, ports: QStringList)
-bool copyDirectory(src: QString, dest: QString)
-QLabel* m_logLevelLabel
-QComboBox* m_logLevelCombo
-QPushButton* m_exportLogBtn
-QPushButton* m_detectPortsBtn
-QPushButton* m_refreshBtn
-QPushButton* m_closeBtn
-QTextEdit* m_resultText
-QTimer* m_detectTimer
}
class CommonUitls {
+static int getAvailablePort()
+static QString ipcServerName(appName: QString)
+static QString getLogDir()
-static QString logDir()
-static bool detailLog()
}
StartWidget --|> QFrame
DebugDialog --|> QDialog
SettingDialog o-- SettingDialogPrivate
SettingDialogPrivate --> CooperationLabel
StartWidget ..> DebugDialog : creates
SettingDialogPrivate ..> DebugDialog : creates
DebugDialog ..> CommonUitls : uses
DebugDialog ..> QTimer : uses
StartWidget ..> QTimer : uses
SettingDialogPrivate ..> QTimer : uses
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 5 issues, and left some high level feedback:
- In both
StartWidgetandSettingDialogPrivate, theclickTimerinterval is set to 3000 ms but the comment still says// 1 second timeout; update the comment or the interval to keep them consistent. - In
DebugDialog,m_detectTimeris created, configured, and connected but never started, so the timeout handling is effectively dead code; either start the timer in the detection flow or remove it. - The
Refreshlogic inDebugDialogcurrently just callsonDetectPorts()again, re-prompting for IP and ports; if the intent is to re-run the last detection, consider caching the last IP/ports and reusing them inonRefreshPorts()instead of asking the user again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both `StartWidget` and `SettingDialogPrivate`, the `clickTimer` interval is set to 3000 ms but the comment still says `// 1 second timeout`; update the comment or the interval to keep them consistent.
- In `DebugDialog`, `m_detectTimer` is created, configured, and connected but never started, so the timeout handling is effectively dead code; either start the timer in the detection flow or remove it.
- The `Refresh` logic in `DebugDialog` currently just calls `onDetectPorts()` again, re-prompting for IP and ports; if the intent is to re-run the last detection, consider caching the last IP/ports and reusing them in `onRefreshPorts()` instead of asking the user again.
## Individual Comments
### Comment 1
<location path="src/common/debugdialog.cpp" line_range="103-68" />
<code_context>
+ this, &QDialog::accept);
+
+ // Setup detection timer
+ m_detectTimer = new QTimer(this);
+ m_detectTimer->setSingleShot(true);
+ m_detectTimer->setInterval(5000); // 5 second timeout
+ connect(m_detectTimer, &QTimer::timeout, this, [this]() {
+ m_resultText->append(tr("\nPort detection timeout!"));
+ m_detectPortsBtn->setEnabled(true);
+ m_refreshBtn->setEnabled(false);
+ });
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Detection timeout timer is never started, so the timeout logic will never trigger.
m_detectTimer is created and wired up, but never started (e.g. in onDetectPorts or detectPortsInternal), so the timeout handler that appends "Port detection timeout!" and re-enables buttons will never run. Either start/stop the timer around the detection process or remove the timer and its related logic to avoid dead code and confusion.
</issue_to_address>
### Comment 2
<location path="src/lib/data-transfer/core/gui/connect/startwidget.cpp" line_range="27" />
<code_context>
DLOG << "Widget constructor called";
+ clickTimer = new QTimer(this);
+ clickTimer->setSingleShot(true);
+ clickTimer->setInterval(3000); // 1 second timeout
+ connect(clickTimer, &QTimer::timeout, this, &StartWidget::resetClickCount);
+
</code_context>
<issue_to_address>
**nitpick:** Timer interval and comment are inconsistent (3000 ms vs "1 second timeout").
Please align the timer interval with the comment—either change the interval to 1000 ms for a 1-second timeout or update the comment to indicate a 3-second timeout.
</issue_to_address>
### Comment 3
<location path="src/lib/data-transfer/core/gui/connect/startwidget.cpp" line_range="100" />
<code_context>
}
}
+
+bool StartWidget::eventFilter(QObject *obj, QEvent *event)
+{
+ if (obj == titleLabel && event->type() == QEvent::MouseButtonRelease) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the click-sequence detection into a reusable helper class and using it from StartWidget (and SettingDialog) instead of keeping the timer/counter/eventFilter logic inside each widget.
You can keep the feature and significantly reduce per‑widget complexity by extracting the click‑sequence logic into a reusable helper, then reusing it in both `StartWidget` and `SettingDialog`.
### 1. Extract reusable click‑sequence helper
For example, add a small QObject in a common location (`common/clicksequencetrigger.h/.cpp`):
```cpp
// common/clicksequencetrigger.h
#pragma once
#include <QObject>
#include <QPointer>
class QTimer;
class QWidget;
class QMouseEvent;
class ClickSequenceTrigger : public QObject
{
Q_OBJECT
public:
explicit ClickSequenceTrigger(
QWidget *target,
int threshold,
int timeoutMs,
std::function<void()> onTriggered,
QObject *parent = nullptr);
protected:
bool eventFilter(QObject *obj, QEvent *event) override;
private:
void resetClickCount();
QPointer<QWidget> m_target;
QTimer *m_timer;
int m_clickCount = 0;
const int m_threshold;
const int m_timeoutMs;
std::function<void()> m_onTriggered;
};
```
```cpp
// common/clicksequencetrigger.cpp
#include "clicksequencetrigger.h"
#include <QTimer>
#include <QMouseEvent>
#include <QWidget>
ClickSequenceTrigger::ClickSequenceTrigger(
QWidget *target,
int threshold,
int timeoutMs,
std::function<void()> onTriggered,
QObject *parent)
: QObject(parent)
, m_target(target)
, m_timer(new QTimer(this))
, m_threshold(threshold)
, m_timeoutMs(timeoutMs)
, m_onTriggered(std::move(onTriggered))
{
m_timer->setSingleShot(true);
m_timer->setInterval(m_timeoutMs);
connect(m_timer, &QTimer::timeout, this, &ClickSequenceTrigger::resetClickCount);
if (m_target) {
m_target->installEventFilter(this);
}
}
bool ClickSequenceTrigger::eventFilter(QObject *obj, QEvent *event)
{
if (obj == m_target && event->type() == QEvent::MouseButtonRelease) {
auto *mouseEvent = static_cast<QMouseEvent*>(event);
if (mouseEvent->button() == Qt::LeftButton) {
++m_clickCount;
if (m_clickCount >= m_threshold) {
if (m_onTriggered)
m_onTriggered();
resetClickCount();
} else {
m_timer->start();
}
return true;
}
}
return QObject::eventFilter(obj, event);
}
void ClickSequenceTrigger::resetClickCount()
{
m_clickCount = 0;
}
```
### 2. Simplify `StartWidget`
Then `StartWidget` no longer needs `clickTimer`, `clickCount`, `CLICK_THRESHOLD` or its own `eventFilter` override:
```cpp
// startwidget.h (remove clickTimer/clickCount/CLICK_THRESHOLD/eventFilter/resetClickCount)
#include "common/clicksequencetrigger.h"
// ...
private:
QLabel *titleLabel = nullptr;
std::unique_ptr<ClickSequenceTrigger> titleClickTrigger;
// ...
```
```cpp
// startwidget.cpp
StartWidget::StartWidget(QWidget *parent)
: QFrame(parent)
{
DLOG << "Widget constructor called";
initUI();
titleClickTrigger = std::make_unique<ClickSequenceTrigger>(
titleLabel,
/*threshold*/ 5, // or your existing CLICK_THRESHOLD
/*timeoutMs*/ 3000,
[this] { showDebugDialog(); },
this
);
}
```
And keep only the feature logic close to where it belongs:
```cpp
void StartWidget::showDebugDialog()
{
DLOG << "Opening debug dialog";
DebugDialog dialog(this);
dialog.exec();
}
```
### 3. Reuse in `SettingDialog`
You can then apply the same helper in `SettingDialog` instead of duplicating the timer + counter + `eventFilter` pattern:
```cpp
// in SettingDialog constructor or init:
m_titleClickTrigger = std::make_unique<ClickSequenceTrigger>(
m_titleLabel,
5,
3000,
[this] { showDebugDialog(); },
this
);
```
This keeps all behavior intact, removes duplicated low‑level event logic from widgets, and centralizes the click‑sequence behavior in a single, well‑named component.
</issue_to_address>
### Comment 4
<location path="src/lib/cooperation/core/gui/dialogs/settingdialog.cpp" line_range="485" />
<code_context>
bool SettingDialog::eventFilter(QObject *watched, QEvent *event)
{
+ // Check for basic label click
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the secret multi-click handling on basicLabel into a reusable helper class and delegating from eventFilter instead of embedding the logic directly there.
You can reduce the added complexity by encapsulating the “secret click” behavior into a reusable helper and delegating from `eventFilter` instead of inlining the logic.
### 1. Extract a small reusable helper
Create a generic helper that owns the timer and click count and can be reused in `StartWidget` and `SettingDialog`:
```cpp
// secretclickhelper.h
class SecretClickHelper : public QObject
{
Q_OBJECT
public:
explicit SecretClickHelper(QObject *watched, QObject *parent = nullptr)
: QObject(parent)
{
watched->installEventFilter(this);
m_timer.setSingleShot(true);
m_timer.setInterval(3000);
connect(&m_timer, &QTimer::timeout, this, &SecretClickHelper::reset);
}
signals:
void activated();
protected:
bool eventFilter(QObject *watched, QEvent *event) override
{
if (event->type() == QEvent::MouseButtonRelease) {
auto *mouseEvent = static_cast<QMouseEvent *>(event);
if (mouseEvent->button() == Qt::LeftButton) {
if (++m_clickCount >= CLICK_THRESHOLD) {
emit activated();
reset();
} else {
m_timer.start();
}
return true;
}
}
return QObject::eventFilter(watched, event);
}
private:
void reset()
{
m_clickCount = 0;
}
static constexpr int CLICK_THRESHOLD = 5; // or whatever you use
int m_clickCount = 0;
QTimer m_timer;
};
```
### 2. Use the helper in `SettingDialogPrivate`
Wire it up in the private class instead of keeping `clickTimer`, `clickCount`, `resetClickCount`, and `showDebugDialog` there:
```cpp
// settingdialog_p.h
#include "secretclickhelper.h"
class SettingDialogPrivate : public QObject
{
// ...
std::unique_ptr<SecretClickHelper> basicLabelSecretClick;
// remove: QTimer *clickTimer; int clickCount; void resetClickCount(); etc.
};
```
```cpp
// settingdialog.cpp (or .p.cpp)
void SettingDialogPrivate::createBasicWidget()
{
basicLabel = new CooperationLabel(tr("Basic Settings"), q);
// ... existing setup ...
basicLabelSecretClick = std::make_unique<SecretClickHelper>(basicLabel, q);
connect(basicLabelSecretClick.get(), &SecretClickHelper::activated,
this, [this] {
DLOG << "Opening debug dialog from settings";
::DebugDialog dialog(q);
dialog.exec();
});
}
```
### 3. Simplify `SettingDialog::eventFilter`
Once the helper owns the click logic, you can keep `eventFilter` focused on painting:
```cpp
bool SettingDialog::eventFilter(QObject *watched, QEvent *event)
{
// no special-case for basicLabel here anymore
if (event->type() != QEvent::Paint)
return QDialog::eventFilter(watched, event);
QWidget *widget = qobject_cast<QWidget *>(watched);
if (!widget)
return QDialog::eventFilter(watched, event);
QPainter painter(widget);
// ... existing background drawing ...
return QDialog::eventFilter(watched, event);
}
```
You can apply the same `SecretClickHelper` to `StartWidget` to remove the duplicated click-count / timer logic there as well. This keeps the dialog’s `eventFilter` narrowly focused and consolidates the “secret click” behavior in one place.
</issue_to_address>
### Comment 5
<location path="src/lib/cooperation/core/gui/dialogs/settingdialog_p.h" line_range="48" />
<code_context>
void onClipboardShareButtonClicked(bool clicked);
void onFileChoosed(const QString &path);
void reportDeviceStatus(const QString &type, bool status);
+ void resetClickCount();
private:
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the debug click-sequence handling into a dedicated helper QObject so the dialog class only wires it up instead of managing all the timer and counter state itself.
You can reduce the added complexity by extracting the click‑sequence debug logic into a small reusable helper, instead of storing `clickTimer`, `clickCount`, `CLICK_THRESHOLD`, and `basicLabel`-specific behavior directly in each private class.
For example, introduce a dedicated QObject that encapsulates the sequence logic and emits a signal when the threshold is reached:
```cpp
// clicksequencetrigger.h
class ClickSequenceTrigger : public QObject
{
Q_OBJECT
public:
explicit ClickSequenceTrigger(int threshold, int timeoutMs, QObject *parent = nullptr);
void attachTo(QWidget *widget); // e.g. CooperationLabel*
Q_SIGNALS:
void triggered(); // emit when click sequence reached
private Q_SLOTS:
void onClicked();
void reset();
private:
QTimer m_timer;
int m_count = 0;
int m_threshold = 0;
};
```
Usage in `SettingDialogPrivate` becomes focused and removes duplicated state:
```cpp
// settingdialog_p.h
#include "clicksequencetrigger.h"
// ...
private:
void createBasicWidget();
void showDebugDialog();
// ...
ClickSequenceTrigger *debugClickTrigger { nullptr };
```
```cpp
// settingdialog_p.cpp (inside createBasicWidget)
basicLabel = new CooperationLabel(/*...*/);
debugClickTrigger = new ClickSequenceTrigger(/*threshold*/ 10, /*timeoutMs*/ 1500, this);
debugClickTrigger->attachTo(basicLabel);
connect(debugClickTrigger, &ClickSequenceTrigger::triggered,
this, &SettingDialogPrivate::showDebugDialog);
```
Then reuse the same `ClickSequenceTrigger` in `StartWidget` (or any other UI) instead of adding another set of `QTimer*`, `clickCount`, `CLICK_THRESHOLD`, and `resetClickCount()`/`showDebugDialog()` methods. This keeps each private class focused on its own UI state and behavior while centralizing the debug‑click mechanism.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| m_detectPortsBtn = new QPushButton(tr("Detect Ports"), this); | ||
| m_refreshBtn = new QPushButton(tr("Refresh"), this); | ||
| m_refreshBtn->setEnabled(false); |
There was a problem hiding this comment.
issue (bug_risk): Detection timeout timer is never started, so the timeout logic will never trigger.
m_detectTimer is created and wired up, but never started (e.g. in onDetectPorts or detectPortsInternal), so the timeout handler that appends "Port detection timeout!" and re-enables buttons will never run. Either start/stop the timer around the detection process or remove the timer and its related logic to avoid dead code and confusion.
| DLOG << "Widget constructor called"; | ||
| clickTimer = new QTimer(this); | ||
| clickTimer->setSingleShot(true); | ||
| clickTimer->setInterval(3000); // 1 second timeout |
There was a problem hiding this comment.
nitpick: Timer interval and comment are inconsistent (3000 ms vs "1 second timeout").
Please align the timer interval with the comment—either change the interval to 1000 ms for a 1-second timeout or update the comment to indicate a 3-second timeout.
| } | ||
| } | ||
|
|
||
| bool StartWidget::eventFilter(QObject *obj, QEvent *event) |
There was a problem hiding this comment.
issue (complexity): Consider extracting the click-sequence detection into a reusable helper class and using it from StartWidget (and SettingDialog) instead of keeping the timer/counter/eventFilter logic inside each widget.
You can keep the feature and significantly reduce per‑widget complexity by extracting the click‑sequence logic into a reusable helper, then reusing it in both StartWidget and SettingDialog.
1. Extract reusable click‑sequence helper
For example, add a small QObject in a common location (common/clicksequencetrigger.h/.cpp):
// common/clicksequencetrigger.h
#pragma once
#include <QObject>
#include <QPointer>
class QTimer;
class QWidget;
class QMouseEvent;
class ClickSequenceTrigger : public QObject
{
Q_OBJECT
public:
explicit ClickSequenceTrigger(
QWidget *target,
int threshold,
int timeoutMs,
std::function<void()> onTriggered,
QObject *parent = nullptr);
protected:
bool eventFilter(QObject *obj, QEvent *event) override;
private:
void resetClickCount();
QPointer<QWidget> m_target;
QTimer *m_timer;
int m_clickCount = 0;
const int m_threshold;
const int m_timeoutMs;
std::function<void()> m_onTriggered;
};// common/clicksequencetrigger.cpp
#include "clicksequencetrigger.h"
#include <QTimer>
#include <QMouseEvent>
#include <QWidget>
ClickSequenceTrigger::ClickSequenceTrigger(
QWidget *target,
int threshold,
int timeoutMs,
std::function<void()> onTriggered,
QObject *parent)
: QObject(parent)
, m_target(target)
, m_timer(new QTimer(this))
, m_threshold(threshold)
, m_timeoutMs(timeoutMs)
, m_onTriggered(std::move(onTriggered))
{
m_timer->setSingleShot(true);
m_timer->setInterval(m_timeoutMs);
connect(m_timer, &QTimer::timeout, this, &ClickSequenceTrigger::resetClickCount);
if (m_target) {
m_target->installEventFilter(this);
}
}
bool ClickSequenceTrigger::eventFilter(QObject *obj, QEvent *event)
{
if (obj == m_target && event->type() == QEvent::MouseButtonRelease) {
auto *mouseEvent = static_cast<QMouseEvent*>(event);
if (mouseEvent->button() == Qt::LeftButton) {
++m_clickCount;
if (m_clickCount >= m_threshold) {
if (m_onTriggered)
m_onTriggered();
resetClickCount();
} else {
m_timer->start();
}
return true;
}
}
return QObject::eventFilter(obj, event);
}
void ClickSequenceTrigger::resetClickCount()
{
m_clickCount = 0;
}2. Simplify StartWidget
Then StartWidget no longer needs clickTimer, clickCount, CLICK_THRESHOLD or its own eventFilter override:
// startwidget.h (remove clickTimer/clickCount/CLICK_THRESHOLD/eventFilter/resetClickCount)
#include "common/clicksequencetrigger.h"
// ...
private:
QLabel *titleLabel = nullptr;
std::unique_ptr<ClickSequenceTrigger> titleClickTrigger;
// ...// startwidget.cpp
StartWidget::StartWidget(QWidget *parent)
: QFrame(parent)
{
DLOG << "Widget constructor called";
initUI();
titleClickTrigger = std::make_unique<ClickSequenceTrigger>(
titleLabel,
/*threshold*/ 5, // or your existing CLICK_THRESHOLD
/*timeoutMs*/ 3000,
[this] { showDebugDialog(); },
this
);
}And keep only the feature logic close to where it belongs:
void StartWidget::showDebugDialog()
{
DLOG << "Opening debug dialog";
DebugDialog dialog(this);
dialog.exec();
}3. Reuse in SettingDialog
You can then apply the same helper in SettingDialog instead of duplicating the timer + counter + eventFilter pattern:
// in SettingDialog constructor or init:
m_titleClickTrigger = std::make_unique<ClickSequenceTrigger>(
m_titleLabel,
5,
3000,
[this] { showDebugDialog(); },
this
);This keeps all behavior intact, removes duplicated low‑level event logic from widgets, and centralizes the click‑sequence behavior in a single, well‑named component.
| DLOG << "SettingDialog destroyed"; | ||
| } | ||
|
|
||
| bool SettingDialog::eventFilter(QObject *watched, QEvent *event) |
There was a problem hiding this comment.
issue (complexity): Consider extracting the secret multi-click handling on basicLabel into a reusable helper class and delegating from eventFilter instead of embedding the logic directly there.
You can reduce the added complexity by encapsulating the “secret click” behavior into a reusable helper and delegating from eventFilter instead of inlining the logic.
1. Extract a small reusable helper
Create a generic helper that owns the timer and click count and can be reused in StartWidget and SettingDialog:
// secretclickhelper.h
class SecretClickHelper : public QObject
{
Q_OBJECT
public:
explicit SecretClickHelper(QObject *watched, QObject *parent = nullptr)
: QObject(parent)
{
watched->installEventFilter(this);
m_timer.setSingleShot(true);
m_timer.setInterval(3000);
connect(&m_timer, &QTimer::timeout, this, &SecretClickHelper::reset);
}
signals:
void activated();
protected:
bool eventFilter(QObject *watched, QEvent *event) override
{
if (event->type() == QEvent::MouseButtonRelease) {
auto *mouseEvent = static_cast<QMouseEvent *>(event);
if (mouseEvent->button() == Qt::LeftButton) {
if (++m_clickCount >= CLICK_THRESHOLD) {
emit activated();
reset();
} else {
m_timer.start();
}
return true;
}
}
return QObject::eventFilter(watched, event);
}
private:
void reset()
{
m_clickCount = 0;
}
static constexpr int CLICK_THRESHOLD = 5; // or whatever you use
int m_clickCount = 0;
QTimer m_timer;
};2. Use the helper in SettingDialogPrivate
Wire it up in the private class instead of keeping clickTimer, clickCount, resetClickCount, and showDebugDialog there:
// settingdialog_p.h
#include "secretclickhelper.h"
class SettingDialogPrivate : public QObject
{
// ...
std::unique_ptr<SecretClickHelper> basicLabelSecretClick;
// remove: QTimer *clickTimer; int clickCount; void resetClickCount(); etc.
};// settingdialog.cpp (or .p.cpp)
void SettingDialogPrivate::createBasicWidget()
{
basicLabel = new CooperationLabel(tr("Basic Settings"), q);
// ... existing setup ...
basicLabelSecretClick = std::make_unique<SecretClickHelper>(basicLabel, q);
connect(basicLabelSecretClick.get(), &SecretClickHelper::activated,
this, [this] {
DLOG << "Opening debug dialog from settings";
::DebugDialog dialog(q);
dialog.exec();
});
}3. Simplify SettingDialog::eventFilter
Once the helper owns the click logic, you can keep eventFilter focused on painting:
bool SettingDialog::eventFilter(QObject *watched, QEvent *event)
{
// no special-case for basicLabel here anymore
if (event->type() != QEvent::Paint)
return QDialog::eventFilter(watched, event);
QWidget *widget = qobject_cast<QWidget *>(watched);
if (!widget)
return QDialog::eventFilter(watched, event);
QPainter painter(widget);
// ... existing background drawing ...
return QDialog::eventFilter(watched, event);
}You can apply the same SecretClickHelper to StartWidget to remove the duplicated click-count / timer logic there as well. This keeps the dialog’s eventFilter narrowly focused and consolidates the “secret click” behavior in one place.
| void onClipboardShareButtonClicked(bool clicked); | ||
| void onFileChoosed(const QString &path); | ||
| void reportDeviceStatus(const QString &type, bool status); | ||
| void resetClickCount(); |
There was a problem hiding this comment.
issue (complexity): Consider extracting the debug click-sequence handling into a dedicated helper QObject so the dialog class only wires it up instead of managing all the timer and counter state itself.
You can reduce the added complexity by extracting the click‑sequence debug logic into a small reusable helper, instead of storing clickTimer, clickCount, CLICK_THRESHOLD, and basicLabel-specific behavior directly in each private class.
For example, introduce a dedicated QObject that encapsulates the sequence logic and emits a signal when the threshold is reached:
// clicksequencetrigger.h
class ClickSequenceTrigger : public QObject
{
Q_OBJECT
public:
explicit ClickSequenceTrigger(int threshold, int timeoutMs, QObject *parent = nullptr);
void attachTo(QWidget *widget); // e.g. CooperationLabel*
Q_SIGNALS:
void triggered(); // emit when click sequence reached
private Q_SLOTS:
void onClicked();
void reset();
private:
QTimer m_timer;
int m_count = 0;
int m_threshold = 0;
};Usage in SettingDialogPrivate becomes focused and removes duplicated state:
// settingdialog_p.h
#include "clicksequencetrigger.h"
// ...
private:
void createBasicWidget();
void showDebugDialog();
// ...
ClickSequenceTrigger *debugClickTrigger { nullptr };// settingdialog_p.cpp (inside createBasicWidget)
basicLabel = new CooperationLabel(/*...*/);
debugClickTrigger = new ClickSequenceTrigger(/*threshold*/ 10, /*timeoutMs*/ 1500, this);
debugClickTrigger->attachTo(basicLabel);
connect(debugClickTrigger, &ClickSequenceTrigger::triggered,
this, &SettingDialogPrivate::showDebugDialog);Then reuse the same ClickSequenceTrigger in StartWidget (or any other UI) instead of adding another set of QTimer*, clickCount, CLICK_THRESHOLD, and resetClickCount()/showDebugDialog() methods. This keeps each private class focused on its own UI state and behavior while centralizing the debug‑click mechanism.
- Extract only the filename part from the full path to support both relative and absolute path formats - Use QFileInfo to properly handle path separators across different platforms - Replace complex string manipulation with more robust file name extraction logic
deepin pr auto review我已对提供的 1. 语法逻辑改进意见：
2. 代码质量改进意见：
3. 代码性能改进意见：
4. 代码安全改进意见：
总结总体来看，这次代码修改是正向的。主要改进了路径处理的健壮性和安全性，增加了必要的版权信息和错误日志，并优化了资源清理逻辑。 主要建议：
|
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.
Summary by Sourcery
Add a hidden debug dialog accessible from key GUI labels and wire it into shared utilities and plugins.
New Features:
Enhancements:
Build: