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

Make Placeholder a type that can be convered to Painter #5306

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

sjudd
Copy link
Collaborator

@sjudd sjudd commented Oct 2, 2023

This removes the deprecated Composable variant of Placeholder. Users who require that functionality should instead use GlideSubcomposition. It's useful to remove this now because it allows for a much simpler Placholder interface and it unblocks us from moving most of GlideImage into GlideModifier. In turn that will let us expose GlideModifier as a real composable API component.

@sjudd
Copy link
Collaborator Author

sjudd commented Oct 2, 2023

@kanelbulle can you let me know how painful removing the subcomposition variants of placeholder would be? I could keep them around for a while longer as special cases, but it would be nice to remove them.

@sjudd
Copy link
Collaborator Author

sjudd commented Oct 17, 2023

@kanelbulle let me know what you think

@kanelbulle
Copy link
Collaborator

kanelbulle commented Oct 17, 2023

Thanks for checking, I think we can remove it, but it would be nice to have it @deprecated for a month or two first if it's not too much trouble. We have less than 10 usages in a couple of different projects so it's not completely trouble-free.

@sjudd sjudd force-pushed the more_robust_placeholder branch from 9eb82a3 to 6e0f66f Compare November 3, 2023 02:35
@sjudd
Copy link
Collaborator Author

sjudd commented Nov 3, 2023

Thanks for checking, I think we can remove it, but it would be nice to have it @deprecated for a month or two first if it's not too much trouble. We have less than 10 usages in a couple of different projects so it's not completely trouble-free.

Ok it turns out it's kind of annoying to get it to work again with the refactor... so I've left the API in but it doesn't work. I'm a real Android engineer now right?

At least it'll compile when you import it

@sjudd sjudd enabled auto-merge (rebase) November 3, 2023 02:36
This removes the deprecated Composable variant of Placeholder. Users who
require that functionality should instead use GlideSubcomposition. It's
useful to remove this now because it allows for a much simpler
Placholder interface and it unblocks us from moving most of GlideImage
into GlideModifier. In turn that will let us expose GlideModifier as a
real composable API component.
@sjudd sjudd force-pushed the more_robust_placeholder branch from 6e0f66f to 1774819 Compare November 3, 2023 05:54
@sjudd sjudd merged commit c06a85d into master Nov 3, 2023
7 checks passed
@sjudd sjudd deleted the more_robust_placeholder branch November 3, 2023 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants