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

height >> 100%, perhaps just in SwiftUI #51

Closed
AndrewSB opened this issue Nov 28, 2022 · 3 comments
Closed

height >> 100%, perhaps just in SwiftUI #51

AndrewSB opened this issue Nov 28, 2022 · 3 comments

Comments

@AndrewSB
Copy link

hey, i was trying this out and was surprised to see that the default verticalScalingFactor (0.95) seemed to clip outside of the bounds of the view

am i doing something wrong?

Screenshot 2022-11-28 at 12 53 08 PM

audio is https://ytdl.hamsterlabs.de/?url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DZ0Uh3OJCx3o

@dmrschmidt
Copy link
Owner

Hey @AndrewSB,

this is in fact "by design" but probably the least intuitive part of the API. I'm open to suggestions to improve it.

So the issue is, that the waveform can be positioned freely along the y-axis of the view. This was done out of a requirement for my own usage of the library. The verticalScalingFactor is in relation to the height of the view, so that, when the waveform is positioned at the top or bottom, it takes up the full view height at a value of 1.

In your case, the waveform is in the middle. So the full height can easily grow out of bounds. I didn't want to do some magic to change the reference point dynamically. So instead, you'd have to set verticalScalingFactor to a value < 0.5 for it to fit.

I guess the best way to get rid of this confusing part is to simply not make the waveform's y position adjustable and leave that to the caller and the view hierarchy outside.

@AndrewSB
Copy link
Author

AndrewSB commented Nov 28, 2022

oh i see, i understand now. thats fine, maybe just some documentation on that would be nice.

a better developer-experience might be to half the vertical scaling factor when the waveform is middle?

i'll close this though, now that i understand it, it isn't a bug

@dmrschmidt
Copy link
Owner

Yeah that was one consideration I had, but it would also be surprising for the values in between, because the "full height" reference size would need to change in a gradual fashion.

I feel the best way forward is really to just get rid of it and have the end-user position the image in their hierarchy how they want it.

dmrschmidt added a commit that referenced this issue Jan 15, 2023
see #51, #57 for instance

Leave positioning up to the view hierarchy outside now. We'll always draw the full waveform centered,
and if an app requires it to be positioned off-center, the view itself can be moved / clipped instead.

Makes more intuitive sense anyway and simplifies the code.
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

No branches or pull requests

2 participants