paparazzi icon indicating copy to clipboard operation
paparazzi copied to clipboard

New feature, viewOnly, to ignore the device frame

Open swankjesse opened this issue 2 years ago • 5 comments

This works in two ways:

  • Just viewOnly omits the system bars
  • viewOnly with RenderingMode.EXPAND also changes the output size to match the view

Closes: https://github.com/cashapp/paparazzi/issues/37

swankjesse avatar Sep 30 '22 20:09 swankjesse

If ever we do a 2.x, it’d be nice to remove com.android.ide.common.rendering.api.SessionParams.RenderingMode from Paparazzi’s public API and instead add a new thing that encapsulates both viewsOnly and that. As-is the two features are a bit weird as there’s some combinations that don‘t do much.

swankjesse avatar Sep 30 '22 20:09 swankjesse

Some interesting outputs!

This button renders just the pixels we need. It’s still scaled up to be 1000px wide, and that’s bad! I’m hoping to address that in the interceptors follow-up. (https://github.com/cashapp/paparazzi/pull/589) app cash paparazzi_ViewOnlyTest_viewSmallerThanDeviceResolution_full_expand-viewonly

This button word-wraps at the width of the target device. This is using viewsOnly = true with H_SCROLL: app cash paparazzi_ViewOnlyTest_viewLargerThanDeviceResolution_v_scroll-viewonly

This ultrawide button doesn’t word wrap at all. It’s using viewsOnly = true with FULL_EXPAND. app cash paparazzi_ViewOnlyTest_viewLargerThanDeviceResolution_full_expand-viewonly

This button renders in a canvas as if there’s a device frame, cause we’re using RenderMode.NORMAL. I don’t think this is a good combination. app cash paparazzi_ViewOnlyTest_viewSmallerThanDeviceResolution_normal-viewonly

swankjesse avatar Sep 30 '22 20:09 swankjesse

Hey, I also tried to solve this problem in my PR: https://github.com/cashapp/paparazzi/pull/550 So some input from my side:

  • As you said it would be better to remove com.android.ide.common.rendering.api.SessionParams.RenderingMode because we don't control this class and there is missing functionality. I tried to solve it with another class instead just a boolean.
  • I think compose layouts are not affected by your change. Paparazzi.kt Line 216 is still using match_parent for height and width.
  • Our main use case is that we want to test Recycler Viewholders. Therefore we want to shrink the y-axis but not the x-axis to check how the view under test behaves on phone/tablet/fablet/foldables.

BenedictP avatar Oct 11 '22 06:10 BenedictP

This is also fixed by #497 and here are some of my thoughts:

  • I think your usage of RenderingMode is backwards. RenderingMode.SHRINK is documented to "Shrink canvas to the minimum size that is needed to cover the scene", which sounds like your interpretation of EXPAND. I think that KEEP is meant to match the size of the device, EXPAND is meant to allow the view to be rendered larger than the device (to support scrolling), and SHRINK is meant to shrink the canvas to match the view.
  • RenderingMode supports defining EXPAND/KEEP/SHRINK along both horizontal and vertical axes, but you seem to be conflating them here with the updated contentRoot.

rharter avatar Oct 13 '22 13:10 rharter

I’m using EXPAND but setting the initial canvas size to 1x1. That way the snapshot can exceed the bounds of the configured device.

As I see them these are the interesting constraints.

  • Layout constraints. For each x,y dimension we can fix to the device, or do unbounded.
  • System bars on or off. These work best if height and width match the device.

swankjesse avatar Oct 13 '22 19:10 swankjesse