sync: from linuxdeepin/dtkdeclarative #238
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @deepin-ci-robot - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider whether
TapHandleris the correct replacement forMouseAreagiven the different semantics. - The change from
implicitHeight: (!__isFullScreen || __isVisible) ? DS.Style.titleBar.height : 0toimplicitHeight: (!__isFullScreen || __isVisible) ? DS.Style.titleBar.height : 1looks suspicious - is there a reason for this change?
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.
qt6/src/qml/TitleBar.qml
Outdated
| property var __dwindow: Window.window.D.DWindow | ||
| property bool __isFullScreen: Window.window.visibility === Window.FullScreen | ||
| property bool __isVisible: mouseArea.containsMouse | ||
| property bool __isVisible: hovereHandler.hovered |
There was a problem hiding this comment.
nitpick (typo): Consider renaming 'hovereHandler' for clarity.
The identifier 'hovereHandler' seems like it might be a typo. Renaming it to 'hoverHandler' could enhance code readability and consistency.
Suggested implementation:
property bool __isVisible: hoverHandler.hovered
Ensure that all other instances of "hovereHandler" are similarly renamed to "hoverHandler" throughout the file.
deepin-ci-robot
force-pushed
the
sync-pr-468-nosync
branch
3 times, most recently
from
d4b0578 to
02745cc
Compare
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#468
deepin-ci-robot
force-pushed
the
sync-pr-468-nosync
branch
from
02745cc to
ed2fe52
Compare
Synchronize source files from linuxdeepin/dtkdeclarative.
Source-pull-request: linuxdeepin/dtkdeclarative#468
Summary by Sourcery
Replace MouseArea with TapHandler and HoverHandler in TitleBar and DialogTitleBar QML components. This change improves the handling of mouse events, especially right-click events for opening the system window menu.