Skip to content

Conversation

@aholachek
Copy link

Hi, here's a quick summary of the major changes:

  • The map is created in the Template.map.rendered function, and subscribes to Reports to listen for changes
  • I gave the map tab its own route which required me to split the station lists out into their own templates, which can be swapped with the map from within the landing template. (Maybe could have been done more simply? This was my first time using Meteor and I'm not quite sure I understand the best way to do routing).
  • The map updates are throttled to once every 15 seconds.
  • Silver line, mattapan, and commuter rail are currently unsupported due to mismatch between the mbta-ninja metadata and my stops.geojson file (and no geo data in case of the commuter rail).
  • I added two new libraries: d3 (visualization) and leaflet (underlying map).
  • The alert triangles are added when the station has at least one report; they are pointed in roughly the relevant directions, and the user can click on them to open a tooltip that offers more information and a link to the relevant page(s).
  • Just to recap, the reason I think a map would be a good idea is that people will want to know roughly where geographically the reports are occurring in the context of a single line e.g. reports on the Braintree end of the red line, for instance, probably won't impact someone in Davis (at least not immediately...), and the user shouldn't have to click through just to learn that the report is irrelevant in his/her case. In addition many people use multiple lines and it's better not to force them to click through to each line. Finally, in list form some of the lines are a bit overwhelming and hard to parse quickly (e.g. the green line dropdown) and, assuming there are fewer than 10 or 15 reports at any one time, I think it's faster to just glance at the map and see where the reports are clustered.

Please let me know what needs to be changed or improved. I'm happy to work more on it to get it ready for release, this was a fun project that has already taught me a lot about meteor and maps!

@radhikamalik
Copy link
Contributor

Hey @aholachek thanks for submitting the PR! To start, could you please rebase and squash all 3 commits into one and also attach a screenshot of the app as it looks now? Thanks!

@aholachek
Copy link
Author

Here's a sample screenshot:

screen shot 2015-11-05 at 7 02 28 pm

I rebased the commits into just the one.

@JacobEvelyn
Copy link
Contributor

This is really cool!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this indentation changed? The change seems incorrect.

@radhikamalik
Copy link
Contributor

This looks really neat!

I added a few stylistic comments on the code so we ensure it's consistent with the existing code.

I wonder if it makes sense to add a line somewhere (maybe on the map view page?) that this view only supports the main lines (no silver line or commuter rail lines).

Also looking at it on mobile, looks like the map starts out with some unnecessary space at the top above "Oak Grove" (and you have scroll down to see all the lines covered completely). It would be good to get rid of this space at the top so you can see all lines together on the map at a glance.
screen shot 2015-11-14 at 5 42 24 pm

Thoughts?

@radhikamalik
Copy link
Contributor

One more thing: we might want to make the buttons to add the reports common to both views so someone doesn't have to necessarily go back to the list view to create a report.

@aholachek
Copy link
Author

Hi Radhika,
I'm sorry that it took so long for me to reply! Thanks for your comments and suggested fixes. Before I get started making the changes, and maybe a few cosmetic changes to the station markers that I've been thinking about, I just want to confirm that this is a feature that should be added to the project. I went to one of the Tuesday meetups and talked to one of the guys who is a leader of the group and map expert (I've unfortunately forgotten his name); he didn't think the map was necessary. Personally I still think it would be useful, for the reasons I wrote above, but I don't want to force through a feature that has limited utility. I'd love to hear your thoughts.

@modestmaya
Copy link

@patrickgreenwell do you know if (at this point in time) there would be any plans to move ahead with the addition of the map feature? Seems like it could be a great feature!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants