-
Notifications
You must be signed in to change notification settings - Fork 116
feat: resize all images to the same aspect ratio and format #701
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
Conversation
germa89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Quick questions:
- Why transparency is not allowed? I think all the images should be provided without background, and then, using your script, I would create two images, one with background for light theme and another for dark theme. Ideally we would like the users to provide two images (dark and light).. but if not, we could use the transparency stuff (if present), if not, then white/greish?.
- It does not appear in this PR, but is the script being executed as part of the CICD? or is the user who opened the PR who has to run it locally in the PC? Then you should probably check that all the files have that resolution in the CICD. I think that is just more work, I would just resize all the images in the doc build job.
|
I'm realising that the |
Many images do not have the capability to provide a transparent background. We should homogenize the way things look in the landing page. This was brought up yesterday in the meeting... I'd have appreciated it you had mentioned it live 😄 We agreed on this since nobody complained.
We can do it on CI/CD. That works for me. I'll implement that. |
Upss.. I'm very sorry about that! Maybe I was distracted for a bit 😅. IMHO I think transparency is the best ... after the option of having two different images (dark/light theme) but that is not an option with the I dont mind either.. simulation, logo... but I think that whatever we put there, it should be dark/light theme compatible. If you want the libraries to be compliant... then, "no logo, no pyansys homage" rule 🤣 Pinging @MaxJPRey @jorgepiloto |
Can we do all this in a follow up issue? For now I'd like to get the homogeneity in place. Currently it's a mess... I'll move it to run on workflow but after that we should merge it. We can discuss about the transparency issue in a follow up. I'd recommend you open an issue and you bring up the topic in the next meeting @germa89 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Would it be a good idea to add a check job in CI to ensure that every changes to doc/source/_static/thumbnails/*.png follows the requirement ?
Edit: just saw #701 (comment), glad we are all thinking alike haha
Co-authored-by: Sébastien Morais <146729917+SMoraisAnsys@users.noreply.github.com>
Let's see what happens after #701 (comment) gets debated and decided. For now I will be moving this logic to the workflow/docs run so that they are dynamically adapted. |
|
@germa89 @SMoraisAnsys - changed the approach. I decided to embed it into the Sphinx build process since we already had other operations running there. Using Sphinx hooks to leverage the execution and clean up. Please review whenever you have the chance. @germa89 I'd encourage you to sign up in the agenda for next week and create an issue to discuss the transparency, logos topic. I think it's super valid your approach, I like transparency and view changes but we need agreement on that from the entire ecosystem. For me, currently, making things look alike is what's critical and this is the first step (i.e. same size, and background). Transparency is an improvement but we need to align. Many repos don't have the necessary images to make them transparent. |
That's something for the branding team, no? In any case, then I'd guess they'd want to make them somewhat uniform.. and consequently, the landing page would be boring. I think the "simulation / creative" image approach work better here. |
Agreed but then teams have to come up with new images with gray background - which we are not going to tackle in this PR 😄 I'd prefer we have a new issue to discuss all this. |
|
I just opened #704 - let's keep the discussion there if possible and leave this PR for the aspect ratio alignment and white background when transparent (temporary solution). |


Implemented script to automate this process in the future whenever a new thumbnail is added. Transparent background will not be accepted. One time process script - instructions on README.
Closes #699