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

Map/Data should only be for Oakland #13

Closed
johnlinahan opened this issue Jan 23, 2017 · 11 comments
Closed

Map/Data should only be for Oakland #13

johnlinahan opened this issue Jan 23, 2017 · 11 comments

Comments

@johnlinahan
Copy link
Contributor

johnlinahan commented Jan 23, 2017

The leaflet map currently has the entire world. It should only focus on Oakland:

  1. Set a max zoom
  2. Lock the movement within certain bounds

We should filter the data as well, some pins are very far away from Oakland like that one 200 miles off the coast of Ghana.

@RitwikGupta
Copy link
Member

The "Ghana" pin is actually just 0'0", that person had their location off.

@vonbearshark
Copy link
Contributor

We should default all unmarked locations to Cathy or something

@vonbearshark vonbearshark added this to the Data management milestone Jan 23, 2017
@johnlinahan johnlinahan changed the title Map/Data should only be for Oakland (BEGINNER - JavaScript, Leaflet) Map/Data should only be for Oakland Jan 24, 2017
@johnlinahan johnlinahan changed the title (BEGINNER - JavaScript, Leaflet) Map/Data should only be for Oakland (JavaScript, Leaflet) Map/Data should only be for Oakland Jan 24, 2017
@johnlinahan johnlinahan changed the title (JavaScript, Leaflet) Map/Data should only be for Oakland Map/Data should only be for Oakland Jan 25, 2017
@RitwikGupta
Copy link
Member

Closed with 91d9a68

@vonbearshark
Copy link
Contributor

vonbearshark commented Jan 25, 2017

We still want to show only Oakland. We should limit the boundaries as well as the zooming out past default zoom. We should remove any unecessary map tiles that we might reference while we're at it. We'll handle the data being set outside the boundary in another issue #61.

@brlodi
Copy link
Collaborator

brlodi commented Jan 25, 2017

I disagree with constraining to the default zoom. Zoom 12 just about contains all of Oakland on a desktop browser, but on mobile it becomes impossible to see the entire neighborhood (especially if the device is landscape) and I think it's important to be able to view the whole thing at once if the user wants.

Related to really limiting the bounds, because of how the bounding box works we're going to have to do it dynamically by the browser window size. Zooming goes nuts if the zoomed out view would cause an edge of the map to go outside the bounds (it recenters to keep the edges within the bounds). Essentially to keep it working how one might expect, the bounds need to increase slightly as the user zooms out. It would be really nice if Leaflet allowed us to set bounds based on the view center instead of the edges, because that would completely solve this problem, but that's not built in.

There's also the issue of pins that correctly appear outside the boundaries of Oakland, but I'll bring that up in #61.

@vonbearshark
Copy link
Contributor

What if we limited default zoom, but we made sure the default zoom we set was different on mobile and Desktop, and could encompass those whole map of Oakland?

re: bounding box: if the bounding box was set just around Oakland such that the default zoom saw everything, but was almost exactly aligned with the box, and we set maxBoundsViscosity to 1, so you couldn't drag outside the area, and we disallowed zooming out past the default, wouldn't that make sure you could see all of and only Oakland (or just around it)?

@brlodi
Copy link
Collaborator

brlodi commented Jan 25, 2017

It is possible to zoom to contain certain bounds, and when generating a bounding box from a set of pins you can specify a padding value, so hypothetically would could dynamically size the zoom to fit. The one caveat is that we've somewhat crippled Leaflet's built in centering and bounds-keeping functions by overlaying the UI sidebar on the map, so we're going to have to remember to manually adjust after every Leaflet call. That's easy for panning (see this StackOverflow post) but harder for calculating zoom to bounding box.

@brlodi
Copy link
Collaborator

brlodi commented Jan 25, 2017

^ which is to say you've convinced me, but the exact implementation will take some work

@vonbearshark
Copy link
Contributor

Yeah I didn't consider that if we disallow panning and contain the area, we might not be able to access some areas due to the UI. This is only a problem on Desktop, because we're making the mobile UI toggle to either take up the whole screen or be hidden completely. Actually, that should work for Desktop, too. So if we want to access some areas, we just hide the UI. Not the best solution, but what do you think?

@vonbearshark
Copy link
Contributor

vonbearshark commented Jan 28, 2017

Also, I think we should zoom in at least once more by default, although we can allow for the zoom level that encompasses Oakland (but no more). We just have a lot of pins now, and it looks a lot better zoomed in

@johnlinahan
Copy link
Contributor Author

👋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants