Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Figure out how to handle layout in GlideNode #5234

Closed
sjudd opened this issue Aug 12, 2023 · 2 comments · Fixed by #5240
Closed

Figure out how to handle layout in GlideNode #5234

sjudd opened this issue Aug 12, 2023 · 2 comments · Fixed by #5240

Comments

@sjudd
Copy link
Collaborator

sjudd commented Aug 12, 2023

See #5230 (comment).

We should verify we're using fixed constraints if they're provided.

If not, ideally we'd find a way of influencing the layout based on the image attributes without overriding fixed constraints and ideally minimizing layout changes when the image or thumbnails load

@sjudd
Copy link
Collaborator Author

sjudd commented Aug 20, 2023

I think there are basically three cases here:

  1. Fixed size
  2. Bounded size (But not fixed)
  3. Unbounded size

Each of these may apply to width or height individually, or both.

There's also some demand (#5219, and useIntrinsicSize in Painter) to adjust the layout based on the image (and/or placeholder) size.

I'm currently thinking we should support:

  1. Fixed size (preferred)
  2. Bounded size
  3. Bounded size with useIntrinsicSize
  4. Unbounded size.

For (1), we'll use the fixed size for both layout and as the target size for the Glide load.
For (2), we'll use the maximum of the bounded size for layout and the target size for the Glide load. The actually drawn image may be significantly smaller than the bounds depending on the request options. This lets you use scales and transformations that fill a space.
For (3), we'll use the maximum of the bounded size for the Glide load, but we'll layout to 0 by default. If a placeholder is set, we'll layout to use the placeholder's size. When the image (and any subsequent thumbnails) load, we'll use those sizes. This is ok for some limited number of use cases (e.g. #5219), but will cause lots of layout jank in others.
For (4), we'll treat this as a request to use the intrinsic size, and match the behavior for (3).

We'll need to add a useIntrinsicSize option to GlideImage to implement (3).

@sjudd
Copy link
Collaborator Author

sjudd commented Aug 20, 2023

I ended up deciding against useIntrinsicSize, I think it differs too much from the Image behavior and it's not super intuitive.

Instead we'll merge (2) and (3) above so that they behave as described, but as if useIntrinsicSize is always true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant