openlibrary icon indicating copy to clipboard operation
openlibrary copied to clipboard

Refactor social image generation code

Open rebecca-shoptaw opened this issue 9 months ago • 0 comments

Prep for project outlined in #9077.

Refactor. A quick but satisfying refactor of the code that determines placement for book covers in the social share image. Done in preparation for a re-use of the code for subject/collection social share images for #9077.

Technical

Note: I'd be happy to change the start_width variable name to something more descriptive/accurate -- start_point? start_x_position? -- if there's a preferred name for it.

The implementation used the mathematical patterns derived from the original version of the function to drastically simplify it:

  • Because the starting point (width) for the first image in the case of 5 images was 63:
if len(image) == 5:
        background.paste(image[0], (63, 174 + max_height - image[0].size[1]))

And the starting point consistently went up by 92 each time the number of images decreased:

elif len(image) == 4:
        background.paste(image[0], (155, 174 + max_height - image[0].size[1]))
 elif len(image) == 3:
        background.paste(image[0], (247, 174 + max_height - image[0].size[1]))

Added a single line to determine the starting point (width) based on the number of images: start_width = 63 + 92 * (5 - len(image))

  • Determined that for each successive image that was pasted to the background (no matter how many images total), the starting width increased by 184:
if len(image) == 5:
        background.paste(image[0], (63, 174 + max_height - image[0].size[1]))
        background.paste(image[1], (247, 174 + max_height - image[1].size[1]))
        background.paste(image[2], (431, 174 + max_height - image[2].size[1]))
elif len(image) == 3:
        background.paste(image[0], (247, 174 + max_height - image[0].size[1]))
        background.paste(image[1], (431, 174 + max_height - image[1].size[1]))
        background.paste(image[2], (615, 174 + max_height - image[2].size[1]))
  • Used both above discoveries to replace the existing if/elif block with simple logic that loops through the images, pasting each to the background. It starts at the determined start_width point, and then adds 184 to the start_width at the end of each loop:
for img in image:
        background.paste(img, (start_width, 174 + max_height - img.size[1]))
        start_width += 184

Testing

I found this code difficult to test locally, as I couldn't generate a demo social share image, but it is relatively straightforward to test mathematically, i.e.:

For 4 images:

  • The start_width, as determined in the new version, will be 63 + 92*(5-4) = 155 ✅ Start width in original version if len(image) == 4 is 155
  • The start_width for the second image, as determined in the new version, will be 155 + 184 = 339 ✅ Start width in original version for image[1] if len(image) == 4 is 339

For 1 image:

  • The start_width as determined in the new version will be 63 + 92*(5-1) = 339 ✅ Start width in original version if there's only one image is 431

The same calculations can be performed with any number of images (up to 5, which is the max that will be entered), and should exactly match the original numbers.

Screenshot

Stakeholders

@mekarpeles @cdrini

rebecca-shoptaw avatar May 06 '24 15:05 rebecca-shoptaw