Cropping issue

[Milo123459]

http://i.fionn.live/76763f.png

[lowlighter]

Yeah it may be possible that rendered images are cropped from time to time because SVG renders differently based on OS/browser/environment 😕

You can adjust padding manually to mitigate this issue if needed 👍

[Milo123459]

Got it, thanks, I would close the issue but I'm on phone

[Milo123459]

@lowlighter a suggestion is just making the svg bigger then it has to be?

[lowlighter]

Yeah maybe default config_padding should be higher, but it's pretty hard to adjust it, #33 shows what's increasing SVG size too much will produce. But maybe 8-10% would be better than 6%

It's really hard to find the right compromise because each browser/os really have their own rendering (as it's essentially HTML/CSS), I even sometimes get display issues because Safari is so much behind other browsers like Firefox or Chrome that it's displaying flex box weirdly.

An attempt I made was to provide embedded fonts for the classic template, but it looked really odd because the rest of webpage was using system fonts (which are different across OS) so it didn't fit, plus it was significantly increasing produced metrics file size.

At least it's better now that when metrics was using static formulas 😅

[Milo123459]

[Milo123459]

same issue, i have an idea however.

[Milo123459]

changing padding doesnt fix

[lowlighter]

I'm not remembering whether you had problems with cropped bottom, or just cropped parts inside the SVG (image from first comment is gone) 😅

If that's the latter, it may be a CSS issue instead. Maybe tweaking line-height or div height may solve the problem

[Milo123459]

This is now cutting stuff off at the bottom. So yes, cropped @lowlighter

[Milo123459]

P.S check my profile for an example, scroll down to the bottom

[lowlighter]

This may not be sufficient, maybe try to use something like 10%, 30% (it's width, height format) or an even-higher value for height.

Also, changes on profile page may not be instantaneous (as images are cached by GitHub). Checking directly your github-metrics.svg (or its raw version) may actually display a different result which is the real render (and seems to be fine, at least from my side)

[Milo123459]

I've done both of those and unfortunately, this error persists. How would we fix it?

[lowlighter]

I need to think a bit about it, I'll check it tomorrow

[lowlighter]

Patch probably needs to be done somewhere here:

metrics/source/app/metrics/utils.mjs

Lines 118 to 138 in 882a93d

	//Render through browser and resize height
	const page = await svgresize.browser.newPage()
	await page.setContent(svg, {waitUntil:"load"})
	let mime = "image/svg+xml"
	let {resized, width, height} = await page.evaluate(async padding => {
	//Disable animations
	const animated = !document.querySelector("svg").classList.contains("no-animations")
	if (animated)
	document.querySelector("svg").classList.add("no-animations")
	//Get bounds and resize
	let {y:height, width} = document.querySelector("svg #metrics-end").getBoundingClientRect()
	height = Math.ceil(height*padding.height)
	width = Math.ceil(width*padding.width)
	//Resize svg
	document.querySelector("svg").setAttribute("height", height)
	//Enable animations
	if (animated)
	document.querySelector("svg").classList.remove("no-animations")
	//Result
	return {resized:new XMLSerializer().serializeToString(document.querySelector("svg")), height, width}
	}, padding)

Maybe puppeteer load event is not enough but I'm not sure other options would solve it, as networkidle0 and networkidle0 won't change anything since metrics don't use any external content (images are all base64 encoded).
It would explain why boundingRect is incorrectly reporting size.

Best would probably to debug without headless mode and on a ubuntu system (same environment as GitHub action) check behaviour of #metrics-end tag.

But yeah unfortunately I don't see any other solution for now 😕

[Milo123459]

Sad, I do think this is somewhat high priority

[lowlighter]

Yeah, I'll try to find a solution to patch it quickly

[Milo123459]

Changes title as it makes more sense. See my profile for an example.

[Milo123459]

So this is fixed in the v3.4 release, right?

[lowlighter]

Not sure, that's why I didn't link it to v3.4
But hopefully it'll be a bit better

[Milo123459]

Woooohooo! Fixed 🗡️