-
Notifications
You must be signed in to change notification settings - Fork 70
Refactor Recording Layers Logic and Rename LayersProxyModel to RecordingLayersProxyModel #3761
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
Pull Request Test Coverage Report for Build 14447401159Details
💛 - Coveralls |
4a0fcd5
to
9ae886d
Compare
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.
Nice!
One more note:
ActiveProject::updateMapSettingsLayers()
is always called right after ActiveProject::updateActiveLayer()
.
If ActiveProject::updateMapSettingsLayers()
was always called first instead, then ActiveProject::updateActiveLayer()
could avoid calling getVisibleLayers()
once more and could just fetch the list of visible layers stored in settings (during ActiveProject::updateMapSettingsLayers()
)
Would that make sense?
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.
Please check the usage of all other members/methods of the proxy model
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.
Tested as suggested: 🟢 :
- Behavior on closing and reopening the app, preferably multiple times
- Behavior on switching projects and adding features of different layer geometry type between switches
- Behavior on adding multiple features across different layers and observing the "Choose Active Layer" panel
- Switching between multiple layers on the "Choose Active Layer" panel
- Ensuring tracking layer is not present on "Choose Active Layer," and performing the tests on both types of projects (with and without position tracking layer)
I once managed to record to the tracking layer manually (which should not be possible) -see empty field on the screenshot, but I cannot replicate that 🤷♂️
Refinement on recording layers handling by:
LayersProxyModel
toRecordingLayersProxyModel
QgsProject
fromRecordingLayersProxyModel
filterFunction
in favor of usingsetFilters
andexceptedLayerIds
recordingAllowed
fromInputUtils
toActiveProject
Active Project
:getVisibleLayers()
andpositionTrackingLayerId()
LayerVisible
) inLayersModel
to support visibility filtering