8000 Refactor Recording Layers Logic and Rename LayersProxyModel to RecordingLayersProxyModel by VitorVieiraZ · Pull Request #3761 · MerginMaps/mobile · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 14 commits into from
Apr 16, 2025

Conversation

VitorVieiraZ
Copy link
Contributor
@VitorVieiraZ VitorVieiraZ commented Mar 24, 2025

Refinement on recording layers handling by:

  • Renaming LayersProxyModel to RecordingLayersProxyModel
  • Decoupling QgsProject from RecordingLayersProxyModel
  • Removing filterFunction in favor of using setFilters and exceptedLayerIds
  • Moving recordingAllowed from InputUtils to ActiveProject
  • Adding new helper methods on Active Project: getVisibleLayers() and positionTrackingLayerId()
  • Adding a new role (LayerVisible) in LayersModel to support visibility filtering

@ValentinBuira ValentinBuira requested a review from uclaros March 24, 2025 13:00
@tomasMizera tomasMizera added this to the 2025.3.0 milestone Mar 24, 2025
@VitorVieiraZ VitorVieiraZ changed the title WIP - RecordingLayersProxyModel WIP - Refactor Recording Layers Logic and Rename LayersProxyModel to RecordingLayersProxyModel Mar 26, 2025
Copy link
github-actions bot commented Mar 26, 2025

Pull Request Test Coverage Report for Build 14447401159

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 71 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.2%) to 60.57%

Files with Coverage Reduction New Missed Lines %
input/app/position/providers/simulatedpositionprovider.cpp 1 90.91%
input/app/layersmodel.h 2 0.0%
input/app/layersmodel.cpp 4 0.0%
input/app/ 8000 inpututils.cpp 23 51.69%
input/app/activeproject.cpp 41 71.83%
Totals Coverage Status
Change from base Build 14308343427: 0.2%
Covered Lines: 8028
Relevant Lines: 13254

💛 - Coveralls

@VitorVieiraZ VitorVieiraZ force-pushed the refactorLayersProxyModel branch from 4a0fcd5 to 9ae886d Compare March 26, 2025 01:06
@VitorVieiraZ VitorVieiraZ changed the title WIP - Refactor Recording Layers Logic and Rename LayersProxyModel to RecordingLayersProxyModel Refactor Recording Layers Logic and Rename LayersProxyModel to RecordingLayersProxyModel Mar 26, 2025
Copy link
Contributor
@uclaros uclaros left a 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?

@VitorVieiraZ VitorVieiraZ modified the milestones: 2025.3.0, 2025.2.0 Mar 28, 2025
@tomasMizera tomasMizera self-requested a review April 9, 2025 11:44
Copy link
Collaborator
@tomasMizera tomasMizera left a 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

@tomasMizera tomasMizera requested a review from harminius April 14, 2025 13:52
Copy link
@harminius harminius left a 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 🤷‍♂️

Screenshot_20250416_101951

@tomasMizera tomasMizera merged commit 32d9995 into master Apr 16, 2025
9 checks passed
@tomasMizera tomasMizera deleted the refactorLayersProxyModel branch April 16, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0