search

FREAC / LABINS

A web mapping application created for the Florida Department of Environmental Protection to provide survey data for the state of Florida.
https://labins.org/map
0 stars 0 forks source link

remove zoom to feature btns when polygon #141

Closed clintonlunn closed 4 years ago

clintonlunn commented 4 years ago

I think it's a bit misleading to the user to have a zoom to feature btn available on a polygon element in the information panel. We don't have logic in the code to zoom to this anymore.

Is this a better reflection of the behavior we want?

In essense, the user now does not get the option to "Zoom to Feature" for polygons, but that functionality exists for other geometry types.

I think we lose some of the interactive functionality of the map this way though. To expand on that, should we instead perform a highlight instead of a zoom?

image

shodge17 commented 4 years ago

I would be OK with highlight rather than the zoom on polygons.

PhilipGriffith commented 4 years ago

I would vote against highlighting polygons. I think that highlighting an object serves to pick out a needle in a haystack of visual data, showing you where the selected object is on the map. When I think of the polygons that would be highlighted on our map, I come up with county boundaries, TRS squares, quad boundaries, etc. But because our data only displays at larger map scales, much of the time there's no question about the location of those objects in the current map view. And when they are selected, they often take up either the entire map area (like county boundaries) or a large part of the screen. In the case of "smaller" polygons, like the TRS grid squares, we provide labels for the purpose of differentiating between polygons.

PhilipGriffith commented 4 years ago

I think my comment was too late. lol

clintonlunn commented 4 years ago

I feel like at the same time, the user can just click the clear btn to clear the selection.

However, that clear button also clears the entire information panel. That is a bit of an unintended consequence.

PhilipGriffith commented 4 years ago

So then that means selecting isn't just selecting: you're adding extra data to the map. Now that I think about it, having a "Zoom To" feature for polygons only from within the information panel may be a way to go. This would create an information panel in which every data element has the same option for navigating to that element's location on the map, but only lines and points get selected, in order to better differentiate them from the lines and points all around them.

So there would still be no zooming when you click on an object on the map, but you could zoom if you pressed the button from within the information panel.

clintonlunn commented 4 years ago

I could get behind that.

As a user, I may not know why I only have an option to highlight the map, but I may want to also zoom to it.

I feel like the behavior we were trying to avoid centered around the user clicking the map and then getting zoomed way out, but if they were to click a button through the information panel, thenthey are making the choice to zoom to that feature.

PhilipGriffith commented 4 years ago

I think you're right that the user has made the choice to zoom if they click the button. In previous iterations when the zoom was obligatory, that wasn't the case and could lead to undesired navigation away from the place of interest.

clintonlunn commented 4 years ago

oops, that didn't actually do what i wanted. still in progress

clintonlunn commented 4 years ago

I have logic that I believe will accomplish what we want as a part of this PR (https://github.com/FREAC/LABINS/pull/124). There, I had separated the zoom to and highlight logic. Rather than going any farther, maybe I should make this change on the enhancements branch and avoid future merge conflicts and writing this code twice.

PhilipGriffith commented 4 years ago

I think separating the "zoom to" and "highlight" logic is a necessary step. It will help the code to be more modular, and (obviously) allows us to do one without the other.

clintonlunn commented 4 years ago

Is this then something we can live with broken until the enhancements branch is moved into the map?

In the interim, should we keep the bevavior in this branch until enhancements is integrated?

PhilipGriffith commented 4 years ago

Enhancements will probably get added in June, if I remember correctly, so it may not matter much. Given that it's a core feature to the UI, it would be nice to just have it fixed, though.

clintonlunn commented 4 years ago

I agree, we should come up with some sort of solution for now. How does the current functionality look in this branch? Is this close to what we want for the time being?

PhilipGriffith commented 4 years ago

I am unable to select anything with the mouse. ???

clintonlunn commented 4 years ago

I don't think this is specific to my branch. I just checked master, tester, and my upgrade branch.

getting this error message: image

PhilipGriffith commented 4 years ago

It looks like their server is down. It's working for me now, though. I think it works well, though could we remove the button for the county boundary and township data? Both of them behave as predicted: the entire screen just goes dark. I'm not really convinced those two data sections should even appear in the same way as the other objects, to be honest, but that's for another discussion. lol Once we remove the buttons from the township and county boundary layers, I think we'll be good to go and I'll integrate it into production.

clintonlunn commented 4 years ago

I wasn't a fan of that functionality either but I do like the ability for the user to highlight the entire county, but the zooming behavior kinda sucks.

I'll remove these buttons today though to avoid this undesireable behavior.

PhilipGriffith commented 4 years ago

I'm going to move this over to a new pull request from this branch to the master branch, just to get on board with the new naming scheme (i.e. remove tester and create production).