-
Notifications
You must be signed in to change notification settings - Fork 63
Handle waveform with peaks
prop as a blank array
#1100
Conversation
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.
Nice solution 👌
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.
LGTM! I did like the suggestion you had before and am curious what your thought process was evaluating the trade offs between this solution and the other you suggested 🙂
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.
Can some tests be included here? I noticed that the waveform component has none and it is important.
I'm okay with tests being added in a separate high-priority issue, in case we want to deploy this change this week or next. |
There is no set date for a release the next week so I don't see a reason to rush here, as long as it is handled before the release as mentioned in the linked issue. |
I can add a couple of tests today and then merge it. Won't delay any releases! |
I don't think we can/should deploy this week anyway, it's Friday after all 🙂 |
[]
as the value of the peaks
proppeaks
prop as a blank array
Fixes
Fixes #1074 by @sarayourfriend
Description
This PR updates the
peaks
prop to use the default value even when given a truthy value of[]
. This prevents the peaks from showing up as blank.Testing Instructions
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin