Skip to content
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

Measurement Tool #3574

Merged
merged 62 commits into from
Oct 22, 2024
Merged

Measurement Tool #3574

merged 62 commits into from
Oct 22, 2024

Conversation

VitorVieiraZ
Copy link
Contributor

@VitorVieiraZ VitorVieiraZ commented Aug 13, 2024

Measurement Tool

RPReplay_Final1727370214.3.1.mov

Enhancements to existing Mergin Maps Components:

  • MMDrawer: Added support for a top-left button with an optional icon.
  • Added support for area formatting in project units within Utils.
  • MMButton now offers size options: Regular (traditional mode) and Small (used in the measurement tool).
  • MMMapHidingLabel (used in showInfoTextMessage in MMMapController) now supports at most three lines.

Resolves #1400

Copy link

github-actions bot commented Aug 13, 2024

Pull Request Test Coverage Report for Build 11441167335

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 488 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.3%) to 60.469%

Files with Coverage Reduction New Missed Lines %
input/app/mmstyle.h 56 1.95%
input/app/main.cpp 85 32.65%
input/app/inpututils.cpp 347 51.67%
Totals Coverage Status
Change from base Build 11327048776: 0.3%
Covered Lines: 7884
Relevant Lines: 13038

💛 - Coveralls

@VitorVieiraZ VitorVieiraZ force-pushed the enhancement/measurementTool branch from b714a1e to af48eba Compare September 4, 2024 22:20
@VitorVieiraZ VitorVieiraZ changed the title WIP - Measurement Tool Measurement Tool Sep 4, 2024
@VitorVieiraZ VitorVieiraZ force-pushed the enhancement/measurementTool branch from 9ca7aec to 60d2968 Compare September 11, 2024 02:44
app/maptools/measurementmaptool.h Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.h Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/qml/map/components/MMMeasureCrosshair.qml Show resolved Hide resolved
Copy link
Contributor

@PeterPetrik PeterPetrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

app/inpututils.cpp Outdated Show resolved Hide resolved
app/inpututils.h Outdated Show resolved Hide resolved
app/qml/components/MMButton.qml Show resolved Hide resolved
@PeterPetrik PeterPetrik added the squash squash before merging label Sep 24, 2024
@tomasMizera tomasMizera added the FROZEN 🥶 do not merge before upcoming release label Sep 26, 2024
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.

Where do you handle the done button? (from the cpp perspective)

app/inpututils.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.h Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/qml/components/MMButton.qml Outdated Show resolved Hide resolved
app/qml/components/MMDrawerHeader.qml Outdated Show resolved Hide resolved
@VitorVieiraZ VitorVieiraZ force-pushed the enhancement/measurementTool branch from c81916e to 8a22005 Compare October 2, 2024 03:35
@tomasMizera tomasMizera removed the FROZEN 🥶 do not merge before upcoming release label Oct 2, 2024
@VitorVieiraZ VitorVieiraZ force-pushed the enhancement/measurementTool branch from 8da3b28 to 78eb80a Compare October 2, 2024 13:47
@tomasMizera
Copy link
Collaborator

Please merge master here so the iOS is running :)

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.

Quite a few comments, but they are rather small. Thanks for addressing my previous ones!

.github/workflows/android.yml Outdated Show resolved Hide resolved
app/inpututils.h Outdated Show resolved Hide resolved
app/inpututils.h Outdated Show resolved Hide resolved
app/qml/gps/MMStakeoutDrawer.qml Show resolved Hide resolved
app/qml/map/components/MMMapHidingLabel.qml Show resolved Hide resolved
gallery/qml/pages/DrawerPage.qml Outdated Show resolved Hide resolved
app/qml/gps/MMMeasureDrawer.qml Outdated Show resolved Hide resolved
app/qml/dialogs/MMFinishMeasurementDialog.qml Outdated Show resolved Hide resolved
app/qml/components/MMButton.qml Outdated Show resolved Hide resolved
app/qml/main.qml Outdated Show resolved Hide resolved
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.

🚀

@tomasMizera
Copy link
Collaborator

Hi @uclaros, can I ask you to review the usage of QGIS API please? Thanks! You will find it in measurementmaptool.cpp/h

@tomasMizera tomasMizera requested a review from uclaros October 14, 2024 12:22
@erpas
Copy link

erpas commented Oct 15, 2024

When you measure a line (not a closed shape) it still says it is Perimeter instead of just Length

@VitorVieiraZ
Copy link
Contributor Author

When you measure a line (not a closed shape) it still says it is Perimeter instead of just Length

Hi @erpas, thanks for the comment. That’s the expected behavior following the tool design

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 work!
I've got a few comments though :)

  • I'd also like to have the units selectable in the measurements panel. While the project default units is a good default start, I can see users wanting to change that temporarily for a single measurement)
  • It would be nice to have 2 decimal digits when the measured distance is small enough (less than a meter? 10 meter? other?)
  • It would be nice if there was a button to toggle closing/opening the geometry. If a user is trying to measure the area of a big square field, we shouldn't force him to re-digitize the first point in order to get an area! It would be way easier to just hit a button after digitizing the 3rd point.

app/inpututils.cpp Outdated Show resolved Hide resolved
app/inpututils.cpp Outdated Show resolved Hide resolved
app/inpututils.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.cpp Outdated Show resolved Hide resolved
app/maptools/measurementmaptool.h Show resolved Hide resolved
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.

Looks good to me! Let me remove the changes to the android CI job

@tomasMizera tomasMizera merged commit 237bd52 into master Oct 22, 2024
1 check passed
@tomasMizera tomasMizera deleted the enhancement/measurementTool branch October 22, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash squash before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add measuring tool for length and area
6 participants