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

Fixes an issue in with placeholder sizing. #84

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

rharter
Copy link
Contributor

@rharter rharter commented Apr 24, 2024

In cases in which the placeholder's intrinsic size is updated, the preview would not be updated to use that size. When using an AsyncImagePainter from Coil, this results in the the Loading state's empty size being used to calculate the placeholder position iff the content size is resolved before the placeholder, causing the placeholder to fill the view.

This change fixes that issue by adding the placeholder's intrinsicSize as a key for the FixedSizePainter's remember call.

)
.build()
)
val source = object : ZoomableImageSource {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly surprised this does not need to be remembered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I suppose that it's not storing any state internally that requires remembering, so even if it gets recreated on recomposition it is still doing the right thing?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I often forget that the compose compiler only looks at functions. From its perspective, resolve() does not require a new composition branch even if the ZoomableImageSource instance is updated.

@saket saket merged commit b9f5dbe into saket:trunk Apr 24, 2024
2 checks passed
@rharter rharter deleted the rh/async_placeholder branch April 24, 2024 21:07
github-merge-queue bot pushed a commit to slackhq/circuit that referenced this pull request Apr 28, 2024
…1373)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[me.saket.telephoto:zoomable-image-coil](https://togithub.com/saket/telephoto)
| dependencies | minor | `0.10.0` -> `0.11.2` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>saket/telephoto
(me.saket.telephoto:zoomable-image-coil)</summary>

###
[`v0.11.2`](https://togithub.com/saket/telephoto/releases/tag/0.11.2)

Bug fixes

-
[saket/telephoto#84:
Fixed an issue causing placeholders to sometimes take up the full screen
(by [@&#8203;rharter](https://togithub.com/rharter))
-
[saket/telephoto#37:
Prevent a crash in `ZoomableAsyncImage()` after the app's disk cache is
cleared
-
[saket/telephoto#83:
Correctly display single-frame GIFs

New changes

- Increase memory cache hit rate with Coil by using `Precision.INEXACT`

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMjYuMiIsInVwZGF0ZWRJblZlciI6IjM3LjMyNi4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
ZacSweers pushed a commit to ZacSweers/CatchUp that referenced this pull request Apr 29, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[me.saket.telephoto:zoomable-image-coil](https://togithub.com/saket/telephoto)
| `0.10.0` -> `0.11.2` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/me.saket.telephoto:zoomable-image-coil/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/me.saket.telephoto:zoomable-image-coil/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/me.saket.telephoto:zoomable-image-coil/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/me.saket.telephoto:zoomable-image-coil/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[me.saket.telephoto:zoomable-image](https://togithub.com/saket/telephoto)
| `0.10.0` -> `0.11.2` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/me.saket.telephoto:zoomable-image/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/me.saket.telephoto:zoomable-image/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/me.saket.telephoto:zoomable-image/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/me.saket.telephoto:zoomable-image/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [me.saket.telephoto:zoomable](https://togithub.com/saket/telephoto) |
`0.10.0` -> `0.11.2` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/me.saket.telephoto:zoomable/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/me.saket.telephoto:zoomable/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/me.saket.telephoto:zoomable/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/me.saket.telephoto:zoomable/0.10.0/0.11.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>saket/telephoto
(me.saket.telephoto:zoomable-image-coil)</summary>

###
[`v0.11.2`](https://togithub.com/saket/telephoto/releases/tag/0.11.2)

Bug fixes

-
[saket/telephoto#84:
Fixed an issue causing placeholders to sometimes take up the full screen
(by [@&#8203;rharter](https://togithub.com/rharter))
-
[saket/telephoto#37:
Prevent a crash in `ZoomableAsyncImage()` after the app's disk cache is
cleared
-
[saket/telephoto#83:
Correctly display single-frame GIFs

New changes

- Increase memory cache hit rate with Coil by using `Precision.INEXACT`

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ZacSweers/CatchUp).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMjEuMiIsInVwZGF0ZWRJblZlciI6IjM3LjMyMS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

2 participants