NYCPlanning / labs-zola

NYC Planning's Zoning and Land Use App
https://zola.planning.nyc.gov
Other
75 stars 26 forks source link

Display 2 Lots Side-By-Side #1190

Open dhochbaum-dcp opened 3 months ago

dhochbaum-dcp commented 3 months ago

Allow users to select 2 lots to perform a side-by-side comparison of them.

image image
jpiacentinidcp commented 2 months ago

Notes from email:

Two issues to resolve:

1) Text for Intersecting Map Layers does not fit within the narrower Tax Lot boxes and will either cross the fold into the adjacent box or get cut off by the edge of the frame, example : Historic Districts

2) Can we explore having the new Icons split into two rows when multiple lots are selected, similarly to how other text features will adjust to fit the narrower width? This should resolve the spacing issue that would otherwise arise by giving each icon more space

dhochbaum-dcp commented 2 months ago

Hi James,

I'm having trouble correcting issue 1, so documenting what I've done in case someone else picks it up. I assume what you mean is this:

Image

But if you meant something else, please let me know. An example of this can be found here: https://deploy-preview-1182--labs-zola.netlify.app/l/lot-comparison/1/1026/7501/1/1111/1?layer-groups=%5B%22tax-lots%22%5D&search=true#13.51/40.78241/-73.9414 The window needs to be resized to be very close to the mobile breakpoint for the issue to occur.

As for issue 2, will keep that in mind when I work on #1187

jpiacentinidcp commented 2 months ago

Hi David, thanks for the update. Yes that is what I was referring to.

Is it the case that the browser window needs to hit the mobile breakpoint, or the width of the individual tax lot description window needs to thin down to its respective width on mobile?

It’s a bit hacky, but can we force or trick the tax lot window to think it’s mobile sized without the browser having to shrink down? I assume we can’t adjust the mobile breakpoint itself without totally screwing up the actual mobile experience.

dhochbaum-dcp commented 2 months ago

So, the screenshot I showed previously is the layout when the width of the window is >= 1024px. When < 1024px, it switches to the mobile view, which looks like this: image

Solutions that push to the mobile view aren't a good idea in this instance because the "Add another tax lot for comparison" button is hidden completely on mobile. If someone were directly linked to a comparison page, it would load and look ok, but it's definitely not optimized for mobile. (Most annoyingly, the text actually wraps properly on mobile in the screenshot above).

Realistically, we probably just need another dev who is better at CSS to figure out why the text wraps properly on mobile but not on desktop.

jpiacentinidcp commented 2 months ago

Thanks for the clarification. It is a good thing for us to check how things will look in mobile to ensure we don't break that view. I think functionality wise it is a good idea to hide the button on mobile but am also glad that if someone is linked a comparison the mobile view will load properly.

Yes, ideally we can ensure that the text wraps properly on desktop to within its own tax lot pane. My amateur guess is that the wrap limits are still referring to the total pane width (which usually only have 1 tax lot) instead of the smaller half width when in the comparison mode.

@TylerMatteo @bmarchena if you have availability this sprint and confidence with CSS this would be great to look into.

dhochbaum-dcp commented 2 months ago

@jpiacentinidcp I've pushed a fix - let me know how it looks for you.

jpiacentinidcp commented 2 months ago

@dhochbaum-dcp Looks to have resolved the issue. Thanks!

TylerMatteo commented 1 month ago

@dhochbaum-dcp @jpiacentinidcp In the process of code reviewing this I found three bugs that I think we should look into:

  1. When you select two lots and close the side panel, the comparison lot is still highlighted on the map

https://github.com/NYCPlanning/labs-zola/assets/9055367/12332ecd-3c58-461e-af91-b1efbc53434c

  1. When you select a zoning district and then close the side panel, the zoning district is still highlighted on the map. I'm not sure how this would be related but it isn't happening in production

https://github.com/NYCPlanning/labs-zola/assets/9055367/0c2a4fb4-72cc-46fb-87ac-f8a131c324a8

  1. If you select a tax lot then select a zoning district, the tax lot is still highlighted on the map. This happens regardless of if you have a comparison lot selected or not. But if you do, only the original lot you selected remains highlighted. Again, confirmed this isn't happening in production

https://github.com/NYCPlanning/labs-zola/assets/9055367/eb4d0481-3833-433f-b1cb-32042d85de23

I'm going to move this back to In Progress until we triage.

dhochbaum-dcp commented 1 month ago

@TylerMatteo I believe I've resolved those bugs with my most recent updates to the branch.

TylerMatteo commented 1 month ago

Confirming they all look resolved to me.