search

timwis / jkan

A lightweight, backend-free open data portal, powered by Jekyll
https://jkan.io
MIT License
219 stars 311 forks source link

Custom hero image #261

Closed lydiascarf closed 1 year ago

lydiascarf commented 1 year ago

Facilitate having a custom image in the hero section

image

netlify[bot] commented 1 year ago

Deploy Preview for jkan-demo ready!

Name Link
Latest commit b0d3eb419aac6360d95d56f6cfbbde3bbbecf704
Latest deploy log https://app.netlify.com/sites/jkan-demo/deploys/64246afe34fb4f00086bc7ef
Deploy Preview https://deploy-preview-261--jkan-demo.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

BryanQuigley commented 1 year ago

Oh, and this is off Phila, but figured it's better to have a sample upstream.

BryanQuigley commented 1 year ago

@timwis as this was already discussed #253 care if I merge things that follow along with that homepage change?

timwis commented 1 year ago

This looks so good!! Great addition.

I've got two suggestions:

  1. At the moment, the hero looks a bit smushed on mobile. Perhaps we could add a utility class to only show the image on large screens? (I think it's d-none d-md-block in bootstrap 5)
  2. Do you think we should make it configurable in _config.yml? (like the logo property) I guess the only advantage would be that admins could turn it off if they don't have/want an image, though maybe that's not necessary.

Lastly, as much as I love Philly, I'd love if we could include a generic, non-city-specific image as the default, to maintain JKAN's ready-out-of-the-box attributes. I suppose that's pretty hard to find with a geospatial visualisation (perhaps if we remove the text labels?), lol. If nothing immediately comes to mind, I'm happy for us to use this one for now, and create an issue asking folk to contribute a more generic default image.

lydiascarf commented 1 year ago

@timwis I added the style and it looks good on mobile now! Thanks for the comment. We need to move on to other cards if we're going to get to everything, so it would be great if we could source the default image from the community. Same issue with configuration. I tried making it work with _config but for some reason the variable didn't populate in the frontend. Not sure if this was a simple issue I couldn't diagnose in time, but I want to make sure I can get to the other home page improvements, so I'm going to leave it as is if that's alright with you!

lydiascarf commented 1 year ago

@timwis nevermind about the config!

BryanQuigley commented 1 year ago

Nothing obvious comes to mind - and now it can be easily changed.