sync: from linuxdeepin/dtkdeclarative #329
Merged
Conversation
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
Contributor
Author
|
[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 |
Reviewer's GuideRefactors the ProgressBar QML implementation to use an opacity-masked, radius-safe filled track and replaces ad‑hoc timer-based highlight animations with a reusable color/position sweep animation component, adjusting both determinate and indeterminate modes and updating the BoxShadow layout. Sequence diagram for reusable light sweep animation in ProgressBarsequenceDiagram
participant ProgressBarImpl
participant DeterminateRect
participant IndeterminateRect
participant LightSweepAnimationComponent
participant DeterminateLightAnimLoader
participant IndeterminateLightAnimLoader
participant LightSweepAnimation
ProgressBarImpl->>DeterminateLightAnimLoader: create Loader(sourceComponent = lightSweepAnimation)
DeterminateLightAnimLoader->>LightSweepAnimationComponent: instantiate
LightSweepAnimationComponent-->>DeterminateLightAnimLoader: instance LightSweepAnimation
DeterminateLightAnimLoader->>LightSweepAnimation: set targetItem = DeterminateRect
DeterminateLightAnimLoader->>LightSweepAnimation: running = true
loop Animation_infinite
par Color_sweep
LightSweepAnimation->>DeterminateRect: gradientColor from highlight to handleGradientColor
LightSweepAnimation->>DeterminateRect: pause
LightSweepAnimation->>DeterminateRect: gradientColor from handleGradientColor to highlight
and Position_sweep
LightSweepAnimation->>DeterminateRect: lightPosition 0 to 1 (InOutSine)
end
LightSweepAnimation->>DeterminateRect: pause
end
Note over ProgressBarImpl,IndeterminateRect: Indeterminate mode uses same Component with different running condition
ProgressBarImpl->>IndeterminateLightAnimLoader: create Loader(sourceComponent = lightSweepAnimation)
IndeterminateLightAnimLoader->>LightSweepAnimationComponent: instantiate
LightSweepAnimationComponent-->>IndeterminateLightAnimLoader: instance LightSweepAnimation
IndeterminateLightAnimLoader->>LightSweepAnimation: set targetItem = IndeterminateRect
ProgressBarImpl->>LightSweepAnimation: running = progressBar.indeterminate && !control.animationStop
Class diagram for refactored ProgressBar QML structureclassDiagram
class ProgressBarImpl {
}
class BoxShadowFilledTrack {
+anchors_fill_parent
+anchors_rightMargin
+shadowOffsetY
+shadowBlur
+cornerRadius
+shadowColor
+visible
}
class DeterminateItem {
+x
+width
+height
}
class DeterminateRect {
+gradientColor
+lightPosition
+width
+height
+radius
+layer_enabled
+clip
}
class DeterminateLightAnimLoader {
+sourceComponent
+item
}
class DeterminateOuterMaskItem {
+layer_enabled
}
class DeterminateOuterMaskRect {
+width
+height
+radius
}
class IndeterminateRect {
+lightPosition
+gradientColor
+radius
}
class IndeterminateLightAnimLoader {
+sourceComponent
+item
}
class LightSweepAnimationComponent {
}
class LightSweepAnimation {
+targetItem
+loops
+running
}
class LightSweepColorAnimationForward {
+target
+property_gradientColor
+from_highlight
+to_handleGradientColor
+duration
}
class LightSweepColorAnimationBackward {
+target
+property_gradientColor
+from_handleGradientColor
+to_highlight
+duration
}
class LightSweepNumberAnimation {
+target
+property_lightPosition
+from
+to
+duration
+easing_InOutSine
}
ProgressBarImpl o-- BoxShadowFilledTrack
ProgressBarImpl o-- DeterminateItem
DeterminateItem o-- DeterminateRect
DeterminateRect o-- DeterminateLightAnimLoader
DeterminateItem o-- DeterminateOuterMaskItem
DeterminateOuterMaskItem o-- DeterminateOuterMaskRect
ProgressBarImpl o-- IndeterminateRect
IndeterminateRect o-- IndeterminateLightAnimLoader
ProgressBarImpl o-- LightSweepAnimationComponent
LightSweepAnimationComponent o-- LightSweepAnimation
LightSweepAnimation o-- LightSweepColorAnimationForward
LightSweepAnimation o-- LightSweepColorAnimationBackward
LightSweepAnimation o-- LightSweepNumberAnimation
DeterminateLightAnimLoader --> LightSweepAnimationComponent : loads
IndeterminateLightAnimLoader --> LightSweepAnimationComponent : loads
LightSweepAnimation --> DeterminateRect : animates
LightSweepAnimation --> IndeterminateRect : animates
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 1 issue, and left some high level feedback:
- Both
Loaderelements are given the idlightAnimLoaderwithin the same QML component, which will cause an id conflict; give them distinct ids (e.g.determinateLightLoaderandindeterminateLightLoader). - The
lightSweepAnimationcomponent assumes itstargetItemdefinesgradientColorandlightPosition; consider adding a null/hasOwnPropertyguard before starting animations inonLoadedto avoid runtime errors if it’s reused elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `Loader` elements are given the id `lightAnimLoader` within the same QML component, which will cause an id conflict; give them distinct ids (e.g. `determinateLightLoader` and `indeterminateLightLoader`).
- The `lightSweepAnimation` component assumes its `targetItem` defines `gradientColor` and `lightPosition`; consider adding a null/`hasOwnProperty` guard before starting animations in `onLoaded` to avoid runtime errors if it’s reused elsewhere.
## Individual Comments
### Comment 1
<location> `qt6/src/qml/private/ProgressBarImpl.qml:58-59` </location>
<code_context>
- moveTimer.interval = 2000
- return;
+
+ Loader {
+ id: lightAnimLoader
+ sourceComponent: lightSweepAnimation
+ onLoaded: {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a binding for `running` instead of setting it imperatively in `onLoaded` so it reacts to state changes.
`lightAnimLoader.item.running` is only set once in `onLoaded`, so changes to `rect.visible` (or other conditions) won’t affect the animation, unlike the previous `Timer { running: rect.visible }` behavior. Move this logic into the animation itself (e.g. bind `running` to `rect.visible && !control.animationStop`), or expose a bindable `running` here instead of assigning it imperatively.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin-ci-robot
force-pushed
the
sync-pr-554-nosync
branch
from
d947446 to
43c762b
Compare
Contributor
Author
deepin pr auto review我来对这段代码进行审查：
component LightSweepAnimation: SequentialAnimation{
property Item targetItem: null
running: targetItem !== null && targetItem.visible
// ... 其他代码
}
component LightSweepAnimation: SequentialAnimation {
id: anim
property Item targetItem: null
property int transitionDuration: 500
property int pauseDuration: 2000
property int sweepDuration: 3000
property color fromColor: progressBar.palette.highlight
property color toColor: control.D.ColorSelector.handleGradientColor
property bool enabled: true
running: enabled && targetItem !== null && targetItem.visible
ParallelAnimation {
SequentialAnimation {
PropertyAnimation {
target: anim.targetItem
property: "gradientColor"
from: anim.fromColor
to: anim.toColor
duration: anim.transitionDuration
}
PauseAnimation { duration: anim.pauseDuration }
PropertyAnimation {
target: anim.targetItem
property: "gradientColor"
from: anim.toColor
to: anim.fromColor
duration: anim.transitionDuration
}
}
NumberAnimation {
target: anim.targetItem
property: "lightPosition"
from: 0
to: 1
duration: anim.sweepDuration
easing.type: Easing.InOutSine
}
}
PauseAnimation { duration: anim.pauseDuration }
}这些改进将使代码更加健壮、可维护，并提供了更好的性能和灵活性。 |
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#554
deepin-ci-robot
force-pushed
the
sync-pr-554-nosync
branch
from
43c762b to
93e7aea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Synchronize source files from linuxdeepin/dtkdeclarative.
Source-pull-request: linuxdeepin/dtkdeclarative#554
Summary by Sourcery
Update the progress bar QML implementation to improve the highlight sweep animation and visual masking for both determinate and indeterminate modes.
Enhancements: