Rework handling of panning to identified features - #4591
Conversation
We will use it to jump to the highlighted feature once we know the available area of the map that is not covered by the panel
- Use the renamed previewPanelHeight property instead of signal params
- Move all highlighting and form opening to a identifyFeature()
- Don't pan if whole feature is near visible map center - Center feature to visible map if it fits - Center clicked location to visible map if whole does not fit
- Add method to calculate the map extent required for a geometry to fit the visible part of the map canvas when covered by a drawer - Use animated zoom when identifying features from a list and not on map
|
might help resolve #3845 as well |
Withalion
left a comment
Withalion
left a comment
There was a problem hiding this comment.
Nice! It would be great to rebase on current master as well since some time passed.
| // Holds the map coordinates of the point the user identified. NaN if identify was triggered from list of features | ||
| property point identifyLocation: Qt.point(NaN, NaN) | ||
|
|
||
| signal featureIdentified( var pair, var point ) |
There was a problem hiding this comment.
| signal featureIdentified( var pair, var point ) | |
| signal featureIdentified( FeatureLayerPair pair, qgsPoint clickedPoint ) |
It's about time we expose FeatureLayerPair & QgsPoint to QML properly
There was a problem hiding this comment.
Well yes, but do you think this PR is the proper place to do it?
| } | ||
| } | ||
|
|
||
| void TestUtilsFunctions::testWhereToPanWhenIdentifying() |
There was a problem hiding this comment.
Could we check here that the scale has not changed as well
There was a problem hiding this comment.
InputUtils::whereToPanWhenIdentifying() does not perform the panning, it only finds out the point that needs to be panned to.
It does not modify the settings at all.
Maybe better const' the InputMapSettings *` param to clarify the intent?
| } | ||
| } | ||
|
|
||
| void TestUtilsFunctions::testDrawerCompensatedExtent() |
There was a problem hiding this comment.
As well here I would have case for:
- when the map is just panned
- when feature is bigger then current extent, so the map zooms out
- when feature is smaller then current extent
There was a problem hiding this comment.
This only tests the extent calculation when compensating for the drawer height.
Testing the actual panning to and zooming to would be a completely different case.
|
@uclaros can you please imrpove the description of this issue. What's happening, what's the issue. Mostly for testers to understand the issue better. |
📦 Build Artifacts Ready
|
2 participants
This PR is a rework on how the map jumps to geometries.
Right now, a double animated pan happens is some cases while the feature form preview drawer is opening.
This is one thing handled by this PR: a single animation is performed while the drawer is opening.
The main change in behavior though is the following:
In all of the above the map canvas scale is unchanged and the pan is animated.
The advantages of this logic are that:
Identifying points or small geometries near the center of the (effective) screen does not modify the extent and the user does not need to re-orient himself (google maps does the same for POIs)
Scale is always preserved while navigating the map.
Identifying large geometries does not modify the map extent too much.
Testing notes: