Anna Duan - Philadelphia ethnic food clusters

[annaduan09]

Storymap project submission

[mjumbewu]

Just a few comments below!

[mjumbewu]

I was surprised that British and Irish dominate the European category, but I guess it's because there are so many pubs, and in my head I lump them into the category of "bar" as distinct from "restaurant". 🤷

[mjumbewu]

I found the timing of map data transitions to be a little disconnected from the slide positions, but when I changed this line in the calcCurrentSlideIndex function from:

const scrollPos = window.scrollY;

to:

const scrollPos = window.scrollY - this.container.offsetTop;

It was a little better. The latter line takes the top position of the slide container element into account. I added that into the template, but it may have been after you forked the code.

[mjumbewu]

suggestion: When using Mapbox's 512px tiles, you should usually set up your tile layer like this. This will reduce the number of tile requests your map has to make to Mapbox, and will prevent your map labels from showing up super tiny.

Suggested change

	maxZoom: 16,
	maxZoom: 16,
	tileSize: 512,
	zoomOffset: -1,

What's happening right now on your map is Mapbox's 512 pixel tiles are being squeezed into Leaflet's default 256x256 pixel tile size, so each tile is getting scaled down to half it's size in each direction. This code is telling leaflet to use 512 pixel tiles instead, but use the tiles from one zoom level offset from what leaflet might expect (i.e. request the zoom level 7 tiles from Mapbox when you're at zoom level 8 on the map).

[mjumbewu]

suggestion: Since you're using the same bindTooltip function for each layer (which is fine) you can do something like this to show different messages for each:

Suggested change

	return `${restaurant} - ${cuisine} - ${cuisine_group}`
	const percent = layer.feature.properties.pct;
	const count = layer.feature.properties.count;
	return (
	// If it's an individual restaurant...
	restaurant ? `${restaurant} - ${cuisine} - ${cuisine_group}` :
	// If it has a percentage...
	percent ? `${cuisine} - ${percent.toFixed(1)}% of ${count} restaurants` :
	// Anything else...
	cuisine
	);

That way you won't get undefined showing up in your tooltips.

[mjumbewu]

suggestion: I might recommend some background on your legend to make the text a little more readable on top of the map tiles.

Suggested change

	.legend {
	.legend {
	padding: 0.5rem;
	border: 1px solid #ccc;
	border-radius: 0.5rem;
	background-color: rgba(255 255 255 / 50%);

[mjumbewu]

suggestion: If you always wanted the colors in the same order (i.e. highest to lowest rank) you could use something like this:

Suggested change

	// Iterate over each unique cuisine to add to the legend
	// Iterate over each unique cuisine to add to the legend
	uniqueCuisines.sort((a, b) => cuisineToRank[a] - cuisineToRank[b]);

[mjumbewu]

suggestion: In order to use Leaflet from an import like this, you have to be using a development server that knows how to deal with it, and you have to build your project to run on any server. This is similar to the vite tool that I showed in class on Wednesday. I'll be re-recording the second half of that class to show how to build a project that uses these imports to deploy to the web, but as long as you include your <script tag in the HTML (i.e., as you do here) then you don't need this import.

Suggested change

import L from 'leaflet';