farmOS / farmOS-map

farmOS-map is an OpenLayers wrapper library designed for agricultural mapping needs. It can be used in any project that has similar requirements.
https://farmOS.github.io/farmOS-map
MIT License
35 stars 21 forks source link

Update map button SVGs #152

Closed paul121 closed 1 year ago

paul121 commented 2 years ago

I'm opening this PR after accidentally closing #147

There were a couple issues with the original code in Edit.js that prevented it from building. I've fixed those and rebased on top of latest 2.x.

paul121 commented 2 years ago

Unfortunately I think this will need some more work. The SVGs are all much larger and have the wrong color.

original

@gbathree I see that all of the SVGs changed this prefix before the path. They all went from a height and width of 24, to a new height and width of 17, most having one or more translations as well. Do you think we can simplify all of this?:

// Polygon example
// Before:
label: '<svg xmlns="http://www.w3.org/2000/svg" height="24" viewBox="0 0 24 24" width="24"><path ....

// After:
label: '<svg width="17mm" height="17mm" version="1.1" viewBox="0 0 17 17" xmlns="http://www.w3.org/2000/svg"><g transform="translate(-103.12 -91.164)"><g stroke-linecap="round" stroke-linejoin="round"><path .......
symbioquine commented 2 years ago

For comparison, here's what the buttons look like currently;

image

symbioquine commented 2 years ago

@gbathree Maybe you can share a bit more detail about the motivation for this change?

Also here's a related issue/comment about a different strategy for organizing these controls - especially since screen space is at a premium: Here's another strategy https://github.com/farmOS/farmOS-map/issues/135#issuecomment-1084847688

symbioquine commented 2 years ago

@paul121 Were you going to ping @gbathree on this?

gbathree commented 2 years ago

sorry all, got behind on emails.

We use this library in SurveyStack as our primary area/point/line drawing tool, and we would consider also using it in soilstack. In both cases, we've had a lot of users (hundreds) use the tool and have accumulated feedback through that process.

I think @paul121 has also seen some of that in working with Regen Farmers Mutual. In general, the feedback we've got is -

The button designs, while elegant, don't fit with users expectations. First, there's a sense that because the fields are perfectly square, that is a square line drawing tool. So there's hesitancy in using it, and rather people use the line tool instead. The problem is that the line tool does not create an area WKT object, and that produces non-obvious cascading user experience failures down the line (you save something that's should be an area as a line and in FarmOS it doesn't show the acreage... when you export to another service, it fails to render it as an area so the service doesn't work right, etc. etc.).

I should also say this doesn't just happen sometimes... it happens a lot.

So the design changes offer a few simple things to address these issues:

  1. The main area icons (circle, poly) look filled, and poly icon is not square. All this helps inform the user that this is for creating general purpose areas, helping them not use inappropriate (ie line) solutions.
  2. The 'modify' tool is just more obvious, showing a pointer dragging a corner. It's less elegant than the previous solution, but in our experience users need clarity over elegance in this situation. It's also very similar to the design of this type of function in other solutions, building on what users may have already seen.

I'm happy to fix the sizing, I think I know the issue now, if this solution feels good.

symbioquine commented 2 years ago

Thanks for taking the time to write that up @gbathree! Those critiques of the existing UX certainly seem on point. (pun intended)

I wasn't doubting that the changes have a legitimate motivation, just pointing out that it would be opaque to someone reading this PR/commit.

In my mind that description/justification belongs in the git commit message itself.

Another thing that should probably go in the commit message is whether the new icons are original artwork by yourself(/who) or copied from somewhere - and if copied what license they're under.

symbioquine commented 1 year ago

Closing in favor of #179

gbathree commented 1 year ago

k good to know. I think I can fix that, thank you for getting back to me!

On Wed, Mar 30, 2022 at 12:33 AM Paul Weidner @.***> wrote:

Unfortunately I think this will need some more work. The SVGs are all much larger and have the wrong color.

[image: original] https://user-images.githubusercontent.com/3116995/160751922-797246d2-6892-4bf7-86a6-d899f5a3521e.png

@gbathree https://github.com/gbathree I see that all of the SVGs changed this prefix before the path. They all went from a height and width of 24, to a new height and width of 17, most having one or more translations as well. Do you think we can simplify all of this?:

// Polygon example // Before: label: '<path ....

// After: label: '<path .......

— Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-map/pull/152#issuecomment-1082611563, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANJMIIHRQDGCO2Z47NJEATVCPKSHANCNFSM5SATYPDQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Greg Austic Check My Calendar https://www.our-sci.net/greg-calendar/ / Availability https://www.our-sci.net/greg-calendar/

+01 919 545 1083 560 Little Lake dr Unit 10 Ann Arbor MI 48103 https://our-sci.net https://bionutrientinstitute.org/