-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[QgsQuick] Add moveTo animation to qgsquick map canvas #50810
[QgsQuick] Add moveTo animation to qgsquick map canvas #50810
Conversation
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.
Much cleaner implementation, thanks.
@@ -330,7 +387,7 @@ Item { | |||
target: null | |||
grabPermissions: PointerHandler.CanTakeOverFromHandlersOfDifferentType | PointerHandler.ApprovesTakeOverByItems | |||
|
|||
onWheel: { | |||
onWheel: function( event ) { |
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.
@tomasMizera , I see you have met Qt6's wise decision to put an end to parameter injections into QML :)
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.
Yeah, took us a lot of time to find all the occurrences though :)
@@ -343,4 +400,8 @@ Item { | |||
userInteractedWithMap() | |||
} | |||
} | |||
|
|||
QgsQuick.QgsQuickUtils { |
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.
IMHO, I'd go down the road of a singleton right away, it's really meant to be used in other places too and making it a singleton from the get go will be more inviting for others to append useful stuff there.
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.
Makes sense, it is now a singleton class
@tomasMizera , can you address the failing CIs? |
2e457d4
to
178b28b
Compare
@nirvn done now |
@nirvn if you are happy, we will merge this on Monday? |
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.
All good 馃憤
This is a follow-up PR to #50271 as that one was closed because of inactivity (Sorry for that @nirvn and thanks for the review!).
PR adds animation to interactively move the map canvas and addresses comments from the previous PR.
It also adds a new utils class to be used from QML (it could be a singleton in the future if we use it in multiple places).
This is how it looks in Mergin Maps 馃檪
20221107114053248.mp4