-
Notifications
You must be signed in to change notification settings - Fork 55
feat: Lock the height of dock. #1139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
add dbus for control-center add lock interface. Log:
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich 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 |
deepin pr auto review代码审查意见:
|
Reviewer's GuideIntroduces a new “Locked” property throughout the Dock: it’s exposed over DBus, persisted in settings, propagated to the panel and proxy, and wired into the QML UI to disable dragging/resizing and add a lock menu item. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @wjyrich - I've reviewed your changes - here's some feedback:
- The manual edits in the qdbusxml2cpp adaptor files risk being overwritten on regeneration—consider wrapping them in HAND-EDIT markers or extending the generator instead.
- In DockSettings::init(), the fallback branch doesn’t initialize m_locked or emit lockedChanged—ensure you handle the new Locked property there as well.
- Consider toggling the QML menu item text between "Lock Dock" and "Unlock Dock" based on Panel.locked to make the action clearer.
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.
| m_locked = newLocked; | ||
| m_dockConfig->setValue(keyLocked, m_locked); | ||
| Q_EMIT lockedChanged(m_locked); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Batch config writes via WriteJob instead of immediate setValue
Use addWriteJob(WriteJob::Locked) and rely on checkWriteJob() to handle the write, ensuring consistency with other setters.
| m_locked = newLocked; | |
| m_dockConfig->setValue(keyLocked, m_locked); | |
| Q_EMIT lockedChanged(m_locked); | |
| m_locked = newLocked; | |
| addWriteJob(WriteJob::Locked); | |
| Q_EMIT lockedChanged(m_locked); |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
add dbus for control-center
add lock interface.
Log:
Summary by Sourcery
Add a lock feature for the dock to prevent height changes by introducing a new "locked" boolean property exposed over DBus, persisted in settings, and controllable via the UI
New Features:
Enhancements: