Skip to content
This repository has been archived by the owner on Feb 13, 2021. It is now read-only.

Add overlays on city names #75

Merged
merged 3 commits into from May 7, 2020
Merged

Add overlays on city names #75

merged 3 commits into from May 7, 2020

Conversation

k8reindeer
Copy link
Contributor

@k8reindeer k8reindeer commented May 5, 2020

Links

  • Issue link
  • Documentation links

GIF/Screenshots:

image

Changes:

  • add background behind text over the picture so that you can read it better

How To Test:

  • This will appear for any list component.

Feedback I'm looking for:

... I don't really think this looks that good? Especially because it varies in size with the content? can I get some design feedback on this?

Things left to do before deploying:

  • ...

@vercel
Copy link

vercel bot commented May 5, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/neighborexpress/neighbor-express/eue9iotbx
✅ Preview: https://neighbor-express-git-overlays-on-list.neighborexpress.now.sh

@erinfrey
Copy link
Contributor

erinfrey commented May 6, 2020

Hey!

Feedback:

  • Agree, doesn't look great with the varied sizes. If we keep the overlay, could we have it extend to the edges?
  • We can also consider alternatives to an overlay. For instance, do we prefer that the city name lies above or below the image or is "embedded" in the image similar to if it was an image card. That would also mean the image looks better, since it's not cut-off midway.

@k8reindeer
Copy link
Contributor Author

I don't know what you mean by

"embedded" in the image similar to if it was an image card.
Can you show a visual example?

One source of inspiration that would be especially easy to pull from would be anything that's on the official USWDS layout: https://designsystem.digital.gov/page-templates/

@keithk
Copy link
Contributor

keithk commented May 6, 2020

The way that I've done this in the past is having an element with 100% height on top of the div, that has a gradient-ed background from transparent->black top->bottom, and then move the text there.

Made a super quick figma mockup: https://www.figma.com/file/B7QmTZk8gvbbmXZAHmj5KP/NEX-homepage?node-id=0%3A1

@k8reindeer
Copy link
Contributor Author

Okay, that looks a bit better to me
image
Does it accomplish what was intended from the marketing team?

Comment on lines 25 to 27
<div className="display-flex flex-justify-start flex-align-end padding-2"
style={{ height: "100%", width: "100%", color: "white",
backgroundImage: 'linear-gradient(rgba(0,0,0,0), rgba(0,0,0,1))'}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd personally prefer extracting these inline styles into a List.module.css and importing it, instead of having them inline.

I'd also maybe reduce the black to .8 or .7 opacity, (I had it at .75 in the mockup I think) so that the photo is a little more clear

Copy link
Contributor

Choose a reason for hiding this comment

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

(I know this is also going on super long for a relatively small thing so also feel free to not play around with the opacity haha)

@k8reindeer
Copy link
Contributor Author

@keithk yeah I like .75 better! I... didn't actually know how to read that value out of Figma, whoops!

Copy link
Contributor

@keithk keithk left a comment

Choose a reason for hiding this comment

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

Yessss that looks great 🙏🏼

@k8reindeer k8reindeer merged commit e11fce9 into master May 7, 2020
@k8reindeer k8reindeer deleted the overlays-on-list branch May 7, 2020 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add city name overlay to images on NEX website
3 participants