feature: add the possibility to handle the "loading" state in WaveformView

[alfogrillo]

Problem Currently when using a WaveformView there is apparently no way to know when the "sampling task" is done. This may cause blank spaces in the UI since the loading isn't instant.

Solution WaveformView already has a custom way to style a WindowShape (providing a closure). WindowShape has now a new API isEmpty that allows clients to know if there is still no content. This is similar (although not identical) to what the SwiftUI AsyncImage does. Furthermore this PR makes DefaultShapeStyler public, so that it's available when developers opt to provide a custom closure for styling a WindowShape.

Result

[dmrschmidt]

Thanks for this PR @alfogrillo. This is a great idea!

I hadn't even thought about this so far. imho it would be great if the API aligned with AsyncImage here. So I'll see if I can work based of your PR and slightly adapt it, so it takes a placeholder contentBuilder instead of relying on the isEmpty.

What is the use case for making DefaultShapeStyler public? I don't think making it public is making it more useful, is it? Since it can't be subclassed / extended anyway.

The original idea of it was to have a reasonable default drawing style. If people needed a custom way to design the Waveform, it is now simple enough by using Shape modifiers directly, isn't it? Genuine question, I'm open to making it public if there's a good use case for it that I didn't see.

[alfogrillo]

Thanks for this PR @alfogrillo. This is a great idea!

I'm glad you liked it! :)

I hadn't even thought about this so far. imho it would be great if the API aligned with AsyncImage here.

For what I saw AsyncImage adds this specific init for handling a placeholder. I think we can something similar in WaveformView as well. I was thinking to something like this:

    public init<I, P>(
        audioURL: URL,
        configuration: Waveform.Configuration = Waveform.Configuration(damping: .init(percentage: 0.125, sides: .both)),
        renderer: WaveformRenderer = LinearWaveformRenderer(),
        priority: TaskPriority = .userInitiated,
        @ViewBuilder content: @escaping (WaveformShape) -> I,
        @ViewBuilder placeholder: @escaping () -> P
    ) where Content == _ConditionalContent<I, P>, I : View, P : View {
        self.init(audioURL: audioURL, configuration: configuration, renderer: renderer, priority: priority) { shape in
            if !shape.isEmpty {
                content(shape)
            } else {
                placeholder()
            }
        }
    }

In this case probably WaveformShape.isEmpty doesn't need to be public anymore.

What is the use case for making DefaultShapeStyler public? I don't think making it public is making it more useful, is it? Since it can't be subclassed / extended anyway.

I think its is useful when developers want to provide a UI for the loading state: eg:

if waveformShape.isEmpty {
   ProgressView()
} else {
    // use the default styler
}

But if we can manage to do it internally, probably it won't be helpful anymore.

The original idea of it was to have a reasonable default drawing style. If people needed a custom way to design the Waveform, it is now simple enough by using Shape modifiers directly, isn't it? Genuine question, I'm open to making it public if there's a good use case for it that I didn't see.

Yeah, I got the idea. The only issue I see with the current API is that when people use the custom closure with the WaveformShape, the style inside the Waveform.Configuration doesn't play any role (while developers can expect it) anymore. Maybe the style shouldn't be inside the Waveform.Configuration to make this obvious?

[dmrschmidt]

    public init<I, P>(
        audioURL: URL,
        configuration: Waveform.Configuration = Waveform.Configuration(damping: .init(percentage: 0.125, sides: .both)),
        renderer: WaveformRenderer = LinearWaveformRenderer(),
        priority: TaskPriority = .userInitiated,
        @ViewBuilder content: @escaping (WaveformShape) -> I,
        @ViewBuilder placeholder: @escaping () -> P
    ) where Content == _ConditionalContent<I, P>, I : View, P : View {
        self.init(audioURL: audioURL, configuration: configuration, renderer: renderer, priority: priority) { shape in
            if !shape.isEmpty {
                content(shape)
            } else {
                placeholder()
            }
        }
    }

Yeah, this would be preferred I'd say, to make it feel more familiar and in line with Apple's APIs. Eventually a init(url:scale:transaction:content:)-like initializer would then also make a lot of sense.

What is the use case for making DefaultShapeStyler public? I don't think making it public is making it more useful, is it? Since it can't be subclassed / extended anyway.

I think its is useful when developers want to provide a UI for the loading state: eg:

Ah right. If I'm not missing anything, this could be solved by having both ViewBuilders be optional though, no? If I only want custom placeholder, I'd simply do

WaveformView(audioURL: audioURL, configuration: configuration, placeholder: {
      Text("Loading")
})

The original idea of it was to have a reasonable default drawing style. If people needed a custom way to design the Waveform, it is now simple enough by using Shape modifiers directly, isn't it? Genuine question, I'm open to making it public if there's a good use case for it that I didn't see.

Yeah, I got the idea. The only issue I see with the current API is that when people use the custom closure with the WaveformShape, the style inside the Waveform.Configuration doesn't play any role (while developers can expect it) anymore. Maybe the style shouldn't be inside the Waveform.Configuration to make this obvious?

Yeah that's pretty annoying, I know. I had thought about how to solve this but didn't really see an easy way. The issue being that some styling just requires the entire drawing logic to be different. The drawing logic however is rather complex. In effect. So you'll need a combination of a custom Renderer and then a custom styler.

I got a little stuck spinning my head around how all those requirements could be fit under one API but didn't get anywhere. Especially since the UIImage logic should remain and there should be some easy to use default styling options. I don't expect many people - if anyone tbh - having to create their own Renderer.

[alfogrillo]

Ah right. If I'm not missing anything, this could be solved by having both ViewBuilders be optional though, no?

It seems optional closures don't work well with @ViewBuilder. :-/ I refactored the code so that the init is similar to the one of AsyncImage.

Notice that now:

• WaveformView doesn't have internally an optional closure anymore
• DefaultShapeStyler is internal again
• WindowShape.isEmpty is internal

I like it more in terms of API, although we now have more inits. Wdyt?

[dmrschmidt]

Yeah that API style looks great. More initializers is ok I'd say, considering the vast amount of initializers some of the SwiftUI classes have. I'll have a look at the optionality. I'm a bit time-strapped this week, so I'll have to see where I can squeeze it in :) But I'll try to get to it timely.

And thanks a lot for the contribution again!

[dmrschmidt]

Alright, merged it in :) I had a quick play with marking the ViewBuilders as optional. I got it to work but frankly, I prefer the constructor version. That also appears to solve a warning when using two trailing closures in a better way. With two optional closures, you're need to qualify them by name explicitly to avoid warnings, which makes the API a tad more verbose.

I'll release this soon as 14.1.0. Only want to see if it makes sense to squeeze #81 in with it.

Thanks a lot again, @alfogrillo!

[alfogrillo]

Thanks @dmrschmidt for the quick iterations.

I'll release this soon as 14.1.0. Only want to see if it makes sense to squeeze https://github.com/dmrschmidt/DSWaveformImage/pull/81 in with it.

I can't wait! :)