scale image sizes

[martin-raden]

The images to illustrate the need for version control were not scaled.

I added appropriate width values.

Best Martin

[github-actions]

🆗 Pre-flight checks passed 😃

This pull request has been checked and contains no modified workflow files, spoofing, or invalid commits.

It should be safe to Approve and Run the workflows that need maintainer approval.

[erinmgraham]

Thanks for working on improving the presentation of these diagrams — the progression you’re trying to show makes sense.

I built a local copy and noticed that the first image in the sequence (play-changes.svg) becomes visually distracting with this change. Because the SVGs have very different aspect ratios, scaling them to be roughly similar in width affects the first image disproportionately (screenshot below).

We generally avoid explicit image sizing in lessons and let the CSS handle scaling by default to avoid issues like these and ease maintainability.

I'll wait to see what others think but if scaling these diagrams is preferred, I think a more robust approach would be to adjust the SVGs themselves (e.g. normalising the canvas/viewBox so they have similar proportions). That keeps the lesson markup simple and avoids layout tweaks that fight the theme. Happy for you to go that route if you’d like.

[martin-raden]

Hi,

I am surprised about the rendering differences. I have used this part of your lesson within my own carpetries-based tutorial (with proper citation) https://dr-eberle-zentrum.github.io/intro-to-git-and-github/instructor/01-git-basics.html and it scales just fine:

Personally, I find the images in the original sizes much too big given their low (but important!) information content.

I see your point in the non-scaling strategy, but eventually I would says this is exactly what stylesheets etc are for?!!

Will see if I find the time to alter the svgs instead.

best,
Martin

[martin-raden]

as an alternative you can put a fixed width into the svgs at line 5 (within the head <svg> definition after the viewport):

• versions.svg and merge.svg : line 5 change at the end to viewBox="0 0 384 354" width="300px"
• play-changes.svg : line 5 change at the end to viewBox="0 0 615 196.5" width="470px"

and the files will be scaled to similar dimensions.

Hope that helps,
Martin

[erinmgraham]

Hi Martin,

Thank you for the detailed follow-up — I appreciate you thinking through alternatives.

This is not a discussion about whether the images are too large, it's a discussion about where responsibility for layout lives (ie where the fix should occur).

I am not comfortable with adding fixed widths (either in Markdown or inside the SVGs), as that couples these assets to a particular rendering context which a lesson cannot control. Case in point is the difference in our respective screenshots.

I would be happy if the SVG canvases/viewBoxes were adjusted so they scale consistently under the existing CSS and I think that would be a solid improvement. Otherwise, I’m inclined to keep the current behaviour.

[martin-raden]

Hi,

sure, your repo.

Eventually, I dont see a difference between a absolute change of the viewport (ie the svg-internal coordinate system) and setting the intended svg-width that does the standard scaling of the viewport to the targeted width. Since SVG is a vector graphics, such a scaling is done anyway and intended.

So from my perspective, setting width equals to scaling a pixelgraphics to the respective dimension but without changing the internal coordinate system, which might cause rounding issues and thus results in a distorted image..

Eventually, I went for the svg-width solution in my repo (without the markdown-css-scaling suggested first).

I can rewrite this PR towards this as well if you like. Otherwise I am not offended when this PR is closed without merging. 😉
It's just a suggestion since I, personally, think the images way to big right now. But it's your decision in the end.

Best,
Martin

[erinmgraham]

Asked Carpentries' Workbench team for best practice advice on this and Toby was the only one who piped up with "have certainly used the same approach as in that PR". Merging.