geomoose / gm3

GeoMoose 3.0 Development. Please submit pull requests to the 'main' branch.
https://www.geomoose.org
MIT License
58 stars 59 forks source link

Ensure measure has a default tool #799

Closed theduckylittle closed 1 year ago

theduckylittle commented 1 year ago

Defaults the measure tool to a polygon.

klassenjs commented 1 year ago

Should the default be line (so as to not change behavior if the user leaves it un-configured)? Otherwise looks fine.

elil commented 1 year ago

Measure defaulting to line seems reasonable to me


From: Jim Klassen @.> Sent: Thursday, February 9, 2023 8:27 PM To: geomoose/gm3 @.> Cc: Subscribed @.***> Subject: Re: [geomoose/gm3] Ensure measure has a default tool (PR #799)

Should the default be line (so as to not change behavior if the user leaves it un-configured)? Otherwise looks fine.

— Reply to this email directly, view it on GitHubhttps://github.com/geomoose/gm3/pull/799#issuecomment-1425158254, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABIE7GT5PDJ3R3SNZYMJJKLWWW7UXANCNFSM6AAAAAAUXI62DU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

klassenjs commented 1 year ago

Back to @theduckylittle to merge/edit/whatever.

elil commented 1 year ago

Defaulting to other types is fine too if there is a reason for it. Only mild preference for line since that is what I measure most and maybe previous defaults were.


From: Jim Klassen @.> Sent: Friday, February 10, 2023 12:11 PM To: geomoose/gm3 @.> Cc: Eli Adam @.>; Comment @.> Subject: Re: [geomoose/gm3] Ensure measure has a default tool (PR #799)

Back to @theduckylittlehttps://github.com/theduckylittle to merge/edit/whatever.

— Reply to this email directly, view it on GitHubhttps://github.com/geomoose/gm3/pull/799#issuecomment-1426286135, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABIE7GSCLVHS5WYTFKGZO6DWW2OGDANCNFSM6AAAAAAUXI62DU. You are receiving this because you commented.Message ID: @.***>

tchaddad commented 1 year ago

+1 for defaulting to line so as not to change behaviour

theduckylittle commented 1 year ago

Is back to line...

klassenjs commented 1 year ago

It looks like the "Fix tests" commit broke tests.

theduckylittle commented 1 year ago

Yeah, I was doing some testing in different branches and a local merge got pushed that wasn't complete. Everything is passing now and I've cleaned up the git history.

theduckylittle commented 1 year ago

Also, screen shot for posterity of changes. I updated the styling to fit in with more of the application:

image

chughes-lincoln commented 1 year ago

One minor comment on the measure multiple results - if you measure a result, and hold shift to measure the same result it selects it again measuring it twice (i.e. the total acreage of a 100 acre property shows as 200 acres). Ideally clicking again on the same feature would deselect it rather than allowing you to select the same feature multiple times.