Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

fix: song placeholder icon in player view #52

Merged
merged 1 commit into from
Nov 16, 2023
Merged

fix: song placeholder icon in player view #52

merged 1 commit into from
Nov 16, 2023

Conversation

SuhasDissa
Copy link
Member

fix #49

@SuhasDissa SuhasDissa requested a review from Bnyro October 15, 2023 09:01
contentDescription,
contentScale = contentScale
)
) {
val state2 = painter.state
Copy link
Member

Choose a reason for hiding this comment

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

state1 and state2 would always be equal because they're reacting to changes to painter.state I think? And this looks way too nested to me, I'd prefer to make it simpler if possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the state1 is for the outer(high res) image
And the state2 is for the inner(low res) image

The painter os actually a member of the SubcomposeAsyncImageScope

This first tries to load the high res one, if it fails, this will try loading the low res image.
If all fails, this will fall back to the placeholder image.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see ...
I thought the painter was introduced somewhere above.
Still, I'd prefer a solution with less nesting, e.g. if we had a boolean val isError by remember { mutableStateOf(false) } that we set to true if there's an error and then do

if (isError) {
   Image(//placeholder)
}

In the same if else statement, so that it's less nested and easier to read.

@SuhasDissa SuhasDissa added the help wanted Extra attention is needed label Oct 24, 2023
@Bnyro Bnyro self-assigned this Nov 15, 2023
@Bnyro Bnyro removed the help wanted Extra attention is needed label Nov 16, 2023
@Bnyro Bnyro merged commit 0a75f19 into you-apps:main Nov 16, 2023
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing thumbnail placeholder for local music
2 participants