Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logo-square images are increasingly not squares #14336

Closed
phrawzty opened this issue Jul 12, 2024 · 7 comments · Fixed by #14636
Closed

logo-square images are increasingly not squares #14336

phrawzty opened this issue Jul 12, 2024 · 7 comments · Fixed by #14636

Comments

@phrawzty
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

mattstratton commented Jul 12, 2024

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
Copy link
Collaborator Author

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 added a commit to toshywoshy/devopsdays-web that referenced this issue Jul 13, 2024
* the event page logos render to 236x236, making all logos square devopsdays#14336
* the welcome page shortcode will render the event logo

Signed-off-by: Toshaan Bharvani <[email protected]>
@toshywoshy
Copy link
Contributor

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
Copy link
Collaborator

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. #14462

@toshywoshy
Copy link
Contributor

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.

mattstratton pushed a commit that referenced this issue Sep 13, 2024
* the event page logos render to 236x236, making all logos square #14336
* the welcome page shortcode will render the event logo

Signed-off-by: Toshaan Bharvani <[email protected]>
mattstratton added a commit that referenced this issue Sep 24, 2024
* add resource gen folder to ignore list

Signed-off-by: Toshaan Bharvani <[email protected]>

* change the event logo to render and fit correctly

* the event page logos render to 236x236, making all logos square #14336
* the welcome page shortcode will render the event logo

Signed-off-by: Toshaan Bharvani <[email protected]>

* render organizer images

Signed-off-by: Toshaan Bharvani <[email protected]>

* render sponsor logos

Signed-off-by: Toshaan Bharvani <[email protected]>

* render speaker/program/talk images

Signed-off-by: Toshaan Bharvani <[email protected]>

* update hugo version and ensure extended version

Signed-off-by: Toshaan Bharvani <[email protected]>

* add missing organizer images

Signed-off-by: Toshaan Bharvani <[email protected]>

* add resource gen folder to ignore list

Signed-off-by: Toshaan Bharvani <[email protected]>

* change the event logo to render and fit correctly

* the event page logos render to 236x236, making all logos square #14336
* the welcome page shortcode will render the event logo

Signed-off-by: Toshaan Bharvani <[email protected]>

* render organizer images

Signed-off-by: Toshaan Bharvani <[email protected]>

* render sponsor logos

Signed-off-by: Toshaan Bharvani <[email protected]>

* render speaker/program/talk images

Signed-off-by: Toshaan Bharvani <[email protected]>

* update hugo version and ensure extended version

Signed-off-by: Toshaan Bharvani <[email protected]>

* add missing organizer images

Signed-off-by: Toshaan Bharvani <[email protected]>

* Fix for case of no speaker directory

Signed-off-by: Matty Stratton <[email protected]>

* Fix edge case for no org directory

Signed-off-by: Matty Stratton <[email protected]>

* Make asset pipeline for sponsor images smaller

Signed-off-by: Matty Stratton <[email protected]>

* use alphabet index folder for sponsor images

Signed-off-by: Toshaan Bharvani <[email protected]>

* remove old files

Signed-off-by: Toshaan Bharvani <[email protected]>

* add caching and  buildstats to understand builds

* add caching to prevent rebuilding everything everytime
* enable stats so we can understand the build speed

Signed-off-by: Toshaan Bharvani <[email protected]>

* move all sponsors to the assets folder

* move all static sponsor images to the assets folder
* alphabetize the sorting to use the first letter
* remove several images that generate problems
    - lenovo.png
    - oracle-mysql.png
    - netlabs.png
    - igv.png
    - nexa.png
    - universidad-de-montevideo.png
    - montevideocomm.png
    - idatha.png
  there may be more, but this is the list I found

Signed-off-by: Toshaan Bharvani <[email protected]>

* add missing d assets sponsor images

images not copied from static
* dasa.png
* devops-meetup-capetown.png

Signed-off-by: Toshaan Bharvani <[email protected]>

* add the netlify plugin for resource caching

Signed-off-by: Toshaan Bharvani <[email protected]>

* add assets sponsors '4' folder

Signed-off-by: Toshaan Bharvani <[email protected]>

* add '1' sponsor asset images

Signed-off-by: Toshaan Bharvani <[email protected]>

* add '3' sponsor asset images

Signed-off-by: Toshaan Bharvani <[email protected]>

* add '5' sponsor asset images

Signed-off-by: Toshaan Bharvani <[email protected]>

* add '6' sponsor asset images

Signed-off-by: Toshaan Bharvani <[email protected]>

* add '9' sponsor asset images

Signed-off-by: Toshaan Bharvani <[email protected]>

---------

Signed-off-by: Toshaan Bharvani <[email protected]>
Signed-off-by: Matty Stratton <[email protected]>
Co-authored-by: Matty Stratton <[email protected]>
Co-authored-by: Matty Stratton <[email protected]>
Co-authored-by: Matty Stratton <[email protected]>
@github-project-automation github-project-automation bot moved this from In progress to Done in devopsdays-web updates Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants