devopsdays / devopsdays-web

This is the website for devopsdays
https://www.devopsdays.org
Other
172 stars 664 forks source link

logo-square images are increasingly not squares #14336

Open phrawzty opened 2 months ago

phrawzty commented 2 months ago

Increasingly, logo-square.jpg files are not squares. This breaks the visual consistency of the homepage. I suggest that we start warning people about this, and as of some reasonable point in the future (2025?), start enforcing it.

For example, we add a check that does something like:

# pass logo-square.jpg only
dimensions=$(file ${1} | grep -oP '(?<=,\s)\d+x\d+')
width=$(echo ${dimensions} | cut -d 'x' -f 1)
height=$(echo ${dimensions} | cut -d 'x' -f 2)
if [[ $width -ne $height ]]; then
    echo "[FAIL] Image is not a square."
    exit 2
fi
if [[ $width -ne 600 ]]; then
    echo "[WARN] Image is not 600x600. See https://github.com/devopsdays/devopsdays-web/blob/main/utilities/README.md#event-square-logo"
    exit 1
fi
exit 0

This uses only standard GNU tooling (no imagemagick or whatever) so it's lightweight.

Initially we don't FAIL. Instead, we raise the warning right in the issue (github hook or whatever) and say "hey you need to fix this by $DATE".

Then on $DATE the check becomes a FAIL.

mattstratton commented 2 months ago

We can do even better, and push the logo square files through Hugo pipes for image processing when Hugo builds the site, and have them cropped to 1:1 ratio.

It’s documented (I believe) that the logo-square needs to be square, so not super worried about a “grace period”.

phrawzty commented 2 months ago

It’s documented (I believe) that the logo-square needs to be square, so not super worried about a “grace period”.

It's documented (see the URL in the sample code above), but we've never enforced it, so just turning it on now would be a pretty terrible experience for our community. Let's make it clear that we're going to enforce this going forward and give people a reasonable period of time to make the adjustment.

mattstratton commented 2 months ago

Cool. We can basically do the CI check as the “warn” and then the image crop filter is the “enforce” which can be enabled after said grace period

Two questions about the warning period:

  1. do we extend until the current events have fallen off the home page since they won’t get caught by the CI check, or do we “manually” reach out to those events and warn them?
  2. How long is the warning period? I propose relatively short (<4 weeks at max)?
phrawzty commented 2 months ago

Cool. We can basically do the CI check as the “warn” and then the image crop filter is the “enforce” which can be enabled after said grace period

Sounds good!

Two questions about the warning period:

  1. I'm guessing we just let current events age out. It's probably(?) more simple from a code perspective, and is also more honest from a community perspective.

  2. I'm European so I'm inclined to give people the summer to sort themselves out. 😆

toshywoshy commented 2 months ago

We can do even better, and push the logo square files through Hugo pipes for image processing when Hugo builds the site, and have them cropped to 1:1 ratio.

So as @mattstratton explains, Hugo extended feature can pipe images and process them, I have created PR #14341 which @mattstratton will expore in detail and hopefully merge after rigerious testing.

However for your reference @phrawzty, I am adding a screenshot of an example of a non-square image, without picking on the event in question. dod-image-fit This image show how Cairo would become square, even though the source is not.

At the moment this is still an optional feature, however it would be made default after some time

bridgetkromhout commented 1 month ago

It's documented (see the URL in the sample code above), but we've never enforced it

Not entirely accurate to say that we've never enforced it; I used to require an adjustment to square before merging a PR (or do it for people), but stopped doing so some time ago.

I'm guessing we just let current events age out. It's probably(?) more simple from a code perspective, and is also more honest from a community perspective.

What I used to do is manually fix it (so people's mistakes wouldn't affect the front page). Here's a PR that fixes the current non-squares; that would buy us enough time to fix it in the pipeline. https://github.com/devopsdays/devopsdays-web/pull/14462

toshywoshy commented 1 month ago

It's documented (see the URL in the sample code above), but we've never enforced it

Not entirely accurate to say that we've never enforced it; I used to require an adjustment to square before merging a PR (or do it for people), but stopped doing so some time ago.

People with the power to merge should check this, and require this before merging, however now the rules seem to be too relaxed in some cases. As for doing it for people, I tend to disagree, it should be the responsability of the PR creator to follow up on that, in the same way it should be on the merger to check this.

I'm guessing we just let current events age out. It's probably(?) more simple from a code perspective, and is also more honest from a community perspective.

What I used to do is manually fix it (so people's mistakes wouldn't affect the front page). Here's a PR that fixes the current non-squares; that would buy us enough time to fix it in the pipeline. #14462

I still think the event in question should create the PR, while adding the merger should have spotted that and insisted to have that corrected before merging. I think that a workflow check as @phrawzty suggests here is a good way to have reviewers have the check automated, however the merger can also spot this easily on the changed files page. My PR would scale/fit the image, however it still may distort images in edge cases, especially if someone were to add too small images or completely disporpotional dimensions.