search

Language-Mapping / language-map

Front-end codebase for Language Mapping web map
https://languagemap.nyc
MIT License
7 stars 4 forks source link

Improve Neighborhood Explore panels #233

Closed abettermap closed 3 years ago

abettermap commented 3 years ago

More or less decoupled all the /Explore/Neighborhood routes from that level to the first child, and left the language profile view intact.

abettermap commented 3 years ago

shoot, i broke this level

image

might have to get at it in the morning, need a walk bad!

rperlin-ela commented 3 years ago

Sure, give the Neighborhood Profiles an escape hatch to Explore/Neighborhoods, I feel you. Yeah, I think I can just do some wordsmithing on the Holliswood-like nabes that have secondaries but no primaries, don’t think they need any special treatment from you. Can I easily get or do I already happen to have internal linkability for that “summary” text too? It’s going to take a big pre-launch push to fill these in credibly, but I’ll push, and I'm grateful for the space.

I do kind of feel like we over-axed the "Show neighborhoods” toggle and need it somewhere a little more prominent. Can’t remember if that’s still in the cards. At least Explore/Neighborhoods itself? Now it’s pretty much only if you find your way to Explore/Nabe/Something, right? And someone might turn it while they’re checking out a Neighborhood and not know where to go turn it off.

On Mar 31, 2021, at 8:17 PM, Jason Lampel @.***> wrote:

shoot, i broke this level

https://user-images.githubusercontent.com/4974087/113226386-4accb980-924d-11eb-949d-1a56f18901eb.png might have to get at it in the morning, need a walk bad!

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/Language-Mapping/language-map/pull/233#issuecomment-811549164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMNKB5EVDKUT57CI3MBYY2DTGO3R3ANCNFSM42F23R6Q.

rperlin-ela commented 3 years ago

Hot stuff on #225. Just made a few comments in that issue.

Not sure about if I see #228 — it might be Gentium but the main Descrip text still looks different than it looked before.

Get out on that walk!

abettermap commented 3 years ago

Sure, give the Neighborhood Profiles an escape hatch to Explore/Neighborhoods, I feel you.

Are we thinking stacky? Like a line below the layer toggle? Or alongside it and with the style of our De-select button in Details? And should that be the button/link wording or something else?

Yeah, I think I can just do some wordsmithing on the Holliswood-like nabes that have secondaries but no primaries, don’t think they need any special treatment from you.

As in change the wording of the generic "additional locations" heading before its list, or the summary in the intro? I'd rather not do a conditional check in the former, and you have control over both, so wordsmith away.

Can I easily get or do I already happen to have internal linkability for that “summary” text too?

I believe so. I think the only ones I robbed you of as far as linkability goes are the search box placeholders.

I do kind of feel like we over-axed the "Show neighborhoods” toggle and need it somewhere a little more prominent. Can’t remember if that’s still in the cards. At least Explore/Neighborhoods itself? Now it’s pretty much only if you find your way to Explore/Nabe/Something, right? And someone might turn it while they’re checking out a Neighborhood and not know where to go turn it off.

Good point. I can add it to /Explore/Neighborhood but I don't think that'll be enough. Honestly the best spot was probably below the Search Locations box, at least with the current UI.

The other alternatives I could think of are so-so but it's gonna be buried no matter what:

abettermap commented 3 years ago

As for these...

Hot stuff on #225. Just made a few comments in that issue.

Not seeing it. Or was it just the "i" button thing? That's the most recent comment I see on that issue.

Not sure about if I see #228 — it might be Gentium but the main Descrip text still looks different than it looked before.

Dammit. I undid the stuff but yeah you're right it's a different font. Gotta be something simple.

abettermap commented 3 years ago

Not sure if you've run into this yet but I'll get on it tomorrow.

image

https://deploy-preview-233--languagemapping.netlify.app/Explore/Neighborhood/Downtown

abettermap commented 3 years ago

Oh and here's the scrollable tabs thing I was suggesting. Haven't tried on desktop but plenty smoove on mobile.

https://material-ui.com/components/tabs/#automatic-scroll-buttons

rperlin-ela commented 3 years ago

Not seeing it. Or was it just the "i" button thing? That's the most recent comment I see on that issue.

My bad, I actually made the comments here. You've already responded to them here below.

Sure, give the Neighborhood Profiles an escape hatch to Explore/Neighborhoods, I feel you.

Are we thinking stacky? Like a line below the layer toggle? Or alongside it and with the style of our De-select button in Details? And should that be the button/link wording or something else?

I think alongside it and with the style of our De-select button in Details, but you do you.

As in change the wording of the generic "additional locations" heading before its list, or the summary in the intro? I'd rather not do a conditional check in the former, and you have control over both, so wordsmith away.

The latter I think, should be nothing for you to worry about

I can add it to /Explore/Neighborhood but I don't think that'll be enough. Honestly the best spot was probably below the Search Locations box, at least with the current UI.

The other alternatives I could think of are so-so but it's gonna be buried no matter what:

Ok, /Explore/Neigborhood for sure. And it can always go back to its former spot below the Search Locations box. The "third tab" option is a little bit intriguing since you were always touting the power of three over there and maybe it would look very clean to have that dumping ground. If you're up for trying!

rperlin-ela commented 3 years ago

Not sure if you've run into this yet but I'll get on it tomorrow.

image

https://deploy-preview-233--languagemapping.netlify.app/Explore/Neighborhood/Downtown

Good catch, my bad, I actually think with borough wired up the name is fine as is (in Mapbox, presumably), so I made the change in AT and now I think it's fine.

abettermap commented 3 years ago

The "third tab" option is a little bit intriguing since you were always touting the power of three over there and maybe it would look very clean to have that dumping ground. If you're up for trying!

I am if I'm right about it being a quick yay or nay. It's either going to fit with "Map Tools" as the tab label on desktop, and feel intuitive on mobile, or it's not. Don't want to fart around with any styling. "Out of the box" defaults from the Material plugin or nada.

abettermap commented 3 years ago

Not sure if you've run into this yet but I'll get on it tomorrow.

image

https://deploy-preview-233--languagemapping.netlify.app/Explore/Neighborhood/Downtown

Good catch, my bad, I actually think with borough wired up the name is fine as is (in Mapbox, presumably), so I made the change in AT and now I think it's fine.

Yep looks good, whew!

abettermap commented 3 years ago

toggle at /Explore/Neighb

neither one is stellar, which one sucks less?

image

image

also, can we say Show neighborhoods in map so it's clear that we're not hiding the list below the toggle?

clear neighbs

speaking of which one sucks less, which of these beauties you want?

image

image

or maybe we've been barking up the wrong tree?

image

it's not as prominent w/longer names:

image

either way, would it get confused with the "X" behavior? and if user does click X, is it ok they lose their clear-ability? (i don't think we need it in popup and panel)

abettermap commented 3 years ago

getting pretty meaty on mobile though

image

rperlin-ela commented 3 years ago

I’m not sure, defer to you, but maybe toggle below (option 2), and “in map” is good.

If we made the move to De-select elsewhere, let’s stick with that. Do we need it on the pop-up? I agree that’s confusing. In the panel it works, and maybe the action can be equivalent to the “x” in the pop-up, but sorry if I missed a beat.

On Apr 1, 2021, at 2:01 PM, Jason Lampel @.***> wrote:

toggle at /Explore/Neighb

neither one is stellar, which one sucks less?

https://user-images.githubusercontent.com/4974087/113333567-cecf8180-92df-11eb-9373-8eb906c53255.png https://user-images.githubusercontent.com/4974087/113333753-148c4a00-92e0-11eb-925e-01c324e00b21.png also, can we say Show neighborhoods in map so it's clear that we're not hiding the list below the toggle?

clear neighbs

speaking of which one sucks less, which of these beauties you want?

https://user-images.githubusercontent.com/4974087/113334548-10acf780-92e1-11eb-8138-22ba93928453.png https://user-images.githubusercontent.com/4974087/113334669-3508d400-92e1-11eb-8f60-2759dd7306db.png or maybe we've been barking up the wrong tree?

https://user-images.githubusercontent.com/4974087/113334932-8add7c00-92e1-11eb-9ed9-9178e805c063.png it's not as prominent w/longer names:

https://user-images.githubusercontent.com/4974087/113335091-b95b5700-92e1-11eb-9063-0da537406b63.png either way, would it get confused with the "X" behavior? and if user does click X, is it ok they lose their clear-ability? (i don't think we need it in popup and panel)

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/Language-Mapping/language-map/pull/233#issuecomment-812075952, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMNKB5B72GNYS7GH3B7DT43TGSYG3ANCNFSM42F23R6Q.

abettermap commented 3 years ago

Do we need it on the pop-up?

not if it's in the panel. was just an alternative.

maybe the action can be equivalent to the “x” in the pop-up, but sorry if I missed a beat

you mean equivalent to X if we do the link in the popup, right? not the panel? panel we decided "de-select" would take you up a level to /Explore/Neighborhood. i think it's moot though, panel is fine.

is left side any better than centered?

image

rperlin-ela commented 3 years ago

Yeah all sounds good. Not sure if left or center is better, both might be ok, your call!

On Apr 1, 2021, at 2:40 PM, Jason Lampel @.***> wrote:

Do we need it on the pop-up?

not if it's in the panel. was just an alternative.

maybe the action can be equivalent to the “x” in the pop-up, but sorry if I missed a beat

you mean equivalent to X if we do the link in the popup, right? not the panel? panel we decided "de-select" would take you up a level to /Explore/Neighborhood. i think it's moot though, panel is fine.

is left side any better than centered?

https://user-images.githubusercontent.com/4974087/113337910-a9de0d00-92e5-11eb-98ac-ac8a965cff08.png — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/Language-Mapping/language-map/pull/233#issuecomment-812097827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMNKB5CDVEG6IK7COUOEOVLTGS42XANCNFSM42F23R6Q.

abettermap commented 3 years ago

so with the Share btn there's a bit too much going on in mobile. ideally everything fits on one line like it does on desktop, but it wraps too much on mobile, but since this is a low priority i'd like to do something generic, so either:

image

or

image

or with a bunch of styling tweaks, could conceivably get on one line:

image

keep in mind the menu opens below:

image

rperlin-ela commented 3 years ago

Totally good — I’m fine with squeezing on one line if possible

On Apr 1, 2021, at 4:19 PM, Jason Lampel @.***> wrote:

so with the Share btn there's a bit too much going on in mobile. ideally everything fits on one line like it does on desktop, but it wraps too much on mobile, but since this is a low priority i'd like to do something generic, so either:

https://user-images.githubusercontent.com/4974087/113348851-62ab4880-92f4-11eb-90ba-403a85cfffa7.png or

https://user-images.githubusercontent.com/4974087/113348952-88385200-92f4-11eb-9bad-d409d5e5e443.png or with a bunch of styling tweaks, could conceivably get on one line:

https://user-images.githubusercontent.com/4974087/113349253-faa93200-92f4-11eb-89e8-f8ee14ff465a.png keep in mind the menu opens below:

https://user-images.githubusercontent.com/4974087/113349371-29270d00-92f5-11eb-9985-b29d4346101e.png — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/Language-Mapping/language-map/pull/233#issuecomment-812148657, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMNKB5GXRY5QVNL26LSHEQLTGTILNANCNFSM42F23R6Q.

abettermap commented 3 years ago

so are all these controls going above or below the summary?

and do you want wrapped like this

image

or back to previous text and just assume they'll figure it out when they click the button?

image

rperlin-ela commented 3 years ago

Ok, maybe we don’t need “in map” so much if it doesn’t quite fit. Below summary seems right

On Apr 1, 2021, at 4:30 PM, Jason Lampel @.***> wrote:

so are all these controls going above or below the summary?

and do you want wrapped like this

https://user-images.githubusercontent.com/4974087/113350382-9be4b800-92f6-11eb-9d07-0a6fc2d898be.png or back to previous text and just assume they'll figure it out when they click the button?

https://user-images.githubusercontent.com/4974087/113350466-be76d100-92f6-11eb-8993-8ac6bfbf92c4.png — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/Language-Mapping/language-map/pull/233#issuecomment-812154399, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMNKB5ESNOSGEF6FXFAJFM3TGTJXNANCNFSM42F23R6Q.

abettermap commented 3 years ago

wowwww spent way to much time on that. definitely give it a look on mobile though. it wouldn't even fit on one line after removing the "Share" text without also hiding the "in map". so smallllllll

anyway we have to finish this friggin branch so please review asap. then i'll start bringing in counties.

make sure to review anything to do with intros in the /Explore/just-about-anything. it's hard to juggle the vertical spacing on certain bits when the stuff below them doesn't occur in all views.

test the #235 thing too please

rperlin-ela commented 3 years ago

Strange, Neighborhoods toggle text now looks like "Show neighborhoodsin map" on desktop, but before (or when I copy the text) I can tell the space is there. Again, I think it's fine just to cut "in map"

abettermap commented 3 years ago

shoot i fixed that, then broke it. it's suppose to preserve the space. will get it in the next branch.