Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add image handling for waveColor and progressColor #2348
add image handling for waveColor and progressColor #2348
Changes from 5 commits
6899409
fbb1735
25d3b51
a62bfd7
13aa47a
f94d789
37fe8a4
d74d8ab
37c73df
5f01b66
f4e25f5
ce75654
8ea3907
bc3134c
9e69788
4fba05a
41b74a8
54c098e
1f567c3
f95472f
61416dc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that we have new methods, let's think about allowing user to change "repeat" value (and other settings in new method). Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an optional options parameter to setWaveColor, setWaveStyle, etc.. that could include options depending on the type of style being set. If it's a gradient, options for controlling the angle of the gradient. If it's a Canvas Image Source, the repeat value.
setWaveStyle(myCanvImgSrc, {repeat: 'repeat-x'});
Are you looking all the way back up to the WaveSurfer parameters object? Or just up to the new methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My recommendation is a new Wavesurfer Params object property called waveFillStyle, which would be an object type that could also be plugged into progressFillStyle, and eventually the wave background, the playhead, etc..)
And keeping waveColor, etc.. as is for backwards compatibility.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's start with
setWaveStyle(myCanvImgSrc, {repeat: 'repeat-x'});
like you suggested. The object passed tosetWaveStyle
can be calledoptions
. We can later decide what this object is called in the main wavesurfer options.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In setWaveStyle, the style is being set to the waveColor parameter, and then drawBuffer is called. How do I get access to this options object over in drawer.canvasentry.js's getFillStyle method?
From what I can tell, I will need to
Is this what you had in mind? It is what I am committing right now, and it works.
If this is not on track with what you were looking for, I will need more details on how else to make this happen. Let me know! Thanks