-
Notifications
You must be signed in to change notification settings - Fork 7
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
uses mapbox-gl and shows all upcoming events #29
Conversation
Code looks great, but I wonder about the user experience of seeing a few early states really busy, and basically nothing elsewhere. Is this better than asking people to put in their location first, and then showing them relevant local or virtual events? Of course the event distribution will change as the campaign moves on from Iowa, but we'll want some way for people across the country to feel engaged. |
@jlev That is a good question. The focus of events is going to be in the early primary states at this point in the campaign. Personally I thought it was great to see events in places like Montana and Alaska, places that aren't close to me so I wouldn't have searched for them. And you're right the distribution of events will change rapidly. I do think you make a great point that we should direct people to virtual events and volunteering to host events themselves. Maybe we can put something like this under the zipcode input / events list:
|
On the topic of redirecting users to create events, we have a technical requirement here to put up a prompt to users if there are no events in their area: #6 |
Would it be possible, on load before a user enters a zip code, to have a rotating marker fading in and fading out of different locations on the map in areas that have events? The fading in and out marker will indicate events happening all over the country, and given that it’ll constantly be changing it doesn’t lead to the perception of “only in x states”. It might also mitigate frontend performance to just be modifying one (or we could have a couple) marker. |
My 2 cents: I think all the styling points here are good ideas! The fading in and out before you type in a zip code seems neat. I also think starting with the full map of events is a pretty dope starting point though :D. I think it's a better initial experience than a blank map imo. I like the look of seeing all the mapped events, personally (though would be good to start zoomed on USA if possible). However, the zip code lookup appears broken in https://mick.github.io/eventmap/ When I type in a zip code, it doesn't change to show much just those nearby events, nor does it zoom me in Strong vote to get to shippable MVP asap! There are volunteers out there who are missing a map-based event search ;) |
Hey, so reviewed the code - and I think I need to go over it again 😅 This PR changes code in multiple components that can possibly impact other features (such as search which is currently broken) and features that are currently being built by other branches. Additionally, I think there are impacts that we can’t visually see right now. For example, I think this breaks the hover-highlight (hovering over an event list item and highlighting the respective venue marker). I totally can concede that the starting code base can be drastically improved, so I just want to go through this again and better understand the pros and cons of the changes made/their impact. But as a result, I don’t feel comfortable merging this today. |
…ch, add link to virtual events and creating new events
Thanks for the comments and feedback everyone! I just committed some more changes and updated https://mick.github.io/eventmap/
I agree. The perf shouldn't a problem with this version of map. So I think we should start with showing all events.
👍 I have the event list now show only the events that are in current map viewport. I added links to virtual events and creating an event at the bottom of the list. So if there are few or no events in the area the user will see it. like:
Yeah I broke that and posted this PR to get feedback on the direction before spending time fixing it. It's reimplemented now. I used the mobilizeamerica api just to get the nearest event to a zipcode and then center the map there.
@joegoldbeck Can you say more here? It should be roughly centered on the USA at zoom level 3 right now. It does remember your location in the url hash. So you can link to a location and if you refresh you'll be in the same location. It could be more zoomed out to start. It could be initially set fit the bounds of the event data.
This should also be fixed now. Although the reverse needs to be clearer (click on a map marker and see the event details). I'll look into that.
@jasonkalmeida Yeah that changes here touch everything. But there are not any other PRs open. Its unclear to me what is still in progress. Can we open draft PRs for branches that have work that is in progress?
Agreed! I've updated https://mick.github.io/eventmap/ Please try it again and list any issues you run into. The biggest outstanding issue is making this useable on mobile #15 |
@mick This looks great! But I am actually seeing significant performance issues. See attached video: WRT the zoom level. It looks better now but when I checked the other day, I saw the whole globe wrapped around :p |
…ing fewer events in list, and using mapbox-gl feature state rather than updating source data. Also highlight and scroll into view an event in the eventlist when clicking on the map
@joegoldbeck whoa yeah that's bad. I just tried it out in Safari and perf was even better than Chrome for me so I'm sure what is up. But there were several things I could do it make it better. I just committed 3 improvements in 111ff45 Can you try that out? I've updated https://mick.github.io/eventmap/ as well.
I also added this. When you click on a map marker it highlights the event in the list and scrolls that event into view if its not already. I think this is close to merge-able. Thoughts? |
Hey @mick Getting this error when I query a zip code |
Does this include multi-event support for a single venue? The current feature on staging is that if a marker is clicked, the event list filters to just the events at that marker - once a user un-selects that marker (by clicking elsewhere/another marker) the event list resets. |
@jasonkalmeida fixed the zipcode c3d1f48
It doesn't as explicitly. When clicking on a marker it centers and zooms in. The list is narrowed down to what's in the viewport of the map. It highlights (just like hover state) and scrolls to the next event at that location. Try it out and lmk if it makes sense. Now that the list is dynamic based on the viewport, it made sense to me. But I can revert that. |
Hey Mick! Sorry for the delays between replies. So a couple things here:
|
To chime in on performance, I ran that test when I had a billion windows open. But, I also have a pretty new MacBook. Developers tend to have decent hardware, but users do not. So I think testing it while your computer is overloaded is a reasonable simulation of what a good fraction of real users will experience. (No point in getting fancy with a tool like this if it’ll make it unusable for even 10% of users IMO) If there have been performance improvements, I can try to test again. |
Ok, it's definitely faster now! |
One thing that feels kind of weird UX-wise is when I type in a zip code, it keeps the old event list while it does a zoom-y thing, and only updates it once it's stopped. Kind of jarring because it looks like maybe it's broken while it's doing the movement / personally I want to see my events right away and don't care so much about having a nice slow swoop to the new location. |
…ction by click elsewhere on the map. clear results when moving map because of zipcode
@jasonkalmeida cool. I changed back to how it worked before. It still filters by viewport, but when a location is selected (by clicking on the marker) it filters to events at that location. When the user clicks off the marker anywhere else on the map it returns to just showing the events in the viewport.
Yeah there are a couple ways things are being filtered. Might be worth adding a "clear selection" button above or below the list. what do you think?
@joegoldbeck Agreed on the perf goals. Thanks for trying it again. Most of the perf gain came from rendering fewer elements in the event list + no longer updating all the map data when highlighting a marker. It uses
@jasonkalmeida yes there is an upper limit, I tested with random generated points, that limit is ~6000 points in view for me, so probably target half that for most users ~3000. We're at ~800 now. At that threshold we could either use clustering, not show all points until users zoom in, or swap back only showing results from mobilzeamerica api. Maybe while keeping some limited initial state to still convey events across the country.
@joegoldbeck yeah totally. I removed the animation and cleared the list on zipcode entry. So it goes blank briefly before it populates with events from the new viewport. Updated: https://mick.github.io/eventmap/ |
Mapbox gl allows for client side rendering of map data, which with a lot of custom markers allows for better performance over leaflet.
This PR relies on the pre-fetched bundle of events in #27. This does change a bit of the map / marker logic. There is a gh-pages deployed version here: https://mick.github.io/eventmap/
TODO
cc @jasonkalmeida @jlev