paparazzi icon indicating copy to clipboard operation
paparazzi copied to clipboard

Add configuration for image size

Open sdoward opened this issue 3 years ago • 7 comments

As per conversation in https://github.com/cashapp/paparazzi/issues/482 this PR adds configuration so consumers can control the size of the images.

Api Considerations

The changes introduces an addition to the api surface. In order to maintain backward compatibility I default to the previous behaviour (limit to 1000px).

However the changes introduce 2 new options...

  • Full bleed images (as per the original discussion)
  • Image size limitation: it seems reasonable to allow consumers to set this limit if we also allow them to choose full bleed images

Testing

I wasn't sure how to write tests for this feature so I added to the sample instead.

Side note: would it make sense to commit images produced in the sample project as some sort of end to end test?

Future Work

The original issue mentioned that we might see issues with report rendering. I took the easier approach here of modifying css. A more solid solution could be to scale the image down when copying during html report generation. I was a little unsure how to do that.

sdoward avatar Aug 13 '22 11:08 sdoward

@sdoward thanks! For me looks good, will wait on @jrodbx's feedback ;)

fcduarte avatar Aug 15 '22 14:08 fcduarte

Hey, guys, any reason why this hasnt been merged? I'm pretty interested in this feature.

micHar avatar Jan 27 '23 18:01 micHar

In case someone wants to use the same feature I've published Paparazzi 1.3.1 with my PR applied: https://github.com/cashapp/paparazzi/pull/567#issuecomment-1671038620

IlyaGulya avatar Aug 09 '23 10:08 IlyaGulya

@fcduarte still waiting for the merge. I need this feature. Any updates?

p-olszewski avatar Oct 15 '23 08:10 p-olszewski

I would also really appreciate merging this feature 🙏

linean avatar Dec 08 '23 15:12 linean

Is there a specific person that can review these changes? Maybe @jrodbx or @TWiStErRob as they've reviewed recent PRs. I mean it looks good to me, but what do I know as a mere user of the lib 😅

npace avatar Dec 11 '23 12:12 npace

I updated the branch and resolved the conflicts

sdoward avatar Dec 11 '23 14:12 sdoward