feat: audio component can be passed transcript text and launch own popup

[jfmcquade]

PR Checklist

• PR title descriptive (can be used in release notes)

Description

Follow up to #2448.

Adds a parameter, transcript_text to the audio component. This parameter takes a string value, which should represent the text of the transcript for the associated audio file. If a value for transcript_text is provided, then the audio component will show the "info button" icon (i.e. show_info_button is set to true), and clicking on this button will automatically launch a pop-up to display the text when the "info button" is clicked, without having to manually author an action, as was the case after #2448 (although custom action handling is still supported).

Git Issues

Closes #

Screenshots/Videos

comp_audio

[github-actions]

Visit the preview URL for this PR (updated for commit 4d0ff66):

https://plh-teens-app1--pr2460-feat-audio-player-co-00nwwtq8.web.app

_{(expires Thu, 24 Oct 2024 10:39:25 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: e4c0bab6b08dd290fbf002fd6e07987fa4b5fce1}

[chrismclarke]

Really nice you were able to get this working so smoothly, I really like the implementation overall and think it should simplify things for authors nicely. See a few minor comments inline, but otherwise I'd say should be good to go

[chrismclarke]

large standard normal - which is it??! 😅

[jfmcquade]

Yeh this is pretty gross... I was using the same pattern to apply those styles as is used by the text component, but now I've updated it so that the pop-up component, when called to display text only, uses an instance of the text component itself. See 50cc535

The text component styling could still do with some tidying up, but at least this means that it's still only in one place

[jfmcquade]

Thanks @chrismclarke, I've addressed your points. I think this should be good to go, pending any opinions you have on my changes to the text component to enable it to be instantiated from within a parent component, see 50cc535

[chrismclarke]

Thanks @chrismclarke, I've addressed your points. I think this should be good to go, pending any opinions you have on my changes to the text component to enable it to be instantiated from within a parent component, see 50cc535

I really like leveraging the existing text component to be able to inherit consistent styling, although would much prefer to isolate the text component from these changes. We've already had to update the common popup component to add support for direct text, I think it starts to feel a bit over-engineered to then also be changing the text component to handle case when being rendered directly within a popup.

So I'd say better to just create an ad-hoc template row in the popup component so that the text can display in the usual way

[chrismclarke]

Possibly too many changes to easily merge as "suggested changes" but hopefully you get the idea at least. Let me know what you think (probably should have done as a PR targetting this branch instead)

[jfmcquade]

So I'd say better to just create an ad-hoc template row in the popup component so that the text can display in the usual way

Ah good idea. I'd forgotten how easy it is to pass in a value for [row]. I've updated this PR to use that pattern, and reverted the changes to the text component (I think I'll make a follow-up PR just refactoring text somewhat)

[chrismclarke]

Looks good to me, thanks for making the updates. Oh yeah, nice idea to tidy up the text component, had thought the same while looking at it

[esmeetewinkel]

Functional test passed