Feat: use parent height if height not set #2841
Conversation
Deploying with
|
| Latest commit: |
faab80e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://efd73922.wavesurfer-js.pages.dev |
| Branch Preview URL: | https://parent-height.wavesurfer-js.pages.dev |
| @@ -93,6 +95,11 @@ class Renderer extends EventEmitter<RendererEvents> { | |||
| ) | |||
| } | |||
|
|
|||
| private getHeight(): number { | |||
| const defaultHeight = 128 | |||
| return this.options.height ?? (this.parent.clientHeight || defaultHeight) | |||
There was a problem hiding this comment.
In your code you are using the default height (128px) only if height doesn't provided in the options and parent.clientHeight is null or undefined
I think it should be: if height isn't provided AND fillParent is true, then don't use the default value
There was a problem hiding this comment.
fillParent was meant only for width. I think it’s sufficient that you either pass a height or you have it fill the container. Otherwise the waveform will be just invisible, which is not very useful, is it?
There was a problem hiding this comment.
I don't think so. I think its very useful for responsiveness in scenarios that the parent height is in relative units and you need the wave to be in the same height as the parent.
There was a problem hiding this comment.
So I’ll leave as it is then. If someone does NOT want to it to fill the full height, they can pass a specific height.
Resolves #2837
It will now use the container height if a height isn't passed in the options, otherwise fall back to 128 px.