st.experimental_audio_input by sfc-gh-nbellante · Pull Request #9404 · streamlit/streamlit

sfc-gh-nbellante · 2024-09-05T01:58:28Z

Describe your changes

🔉 This PR introduces the st.experimental_audio_input widget! 🎤

Light Theme Demo

Dark Theme Demo

Light Theme No Mic Permissions

Dark Theme No Mic Permissions

Note: this currently does not work on safari and by extension iOS so users on that platform will see a warning with the audio_input disabled

GitHub Issue Link (if applicable)

#8899
#903

Testing Plan

Unit Tests (JS and/or Python)
- python unit tests added ✅
E2E Tests
- e2e tests added ✅
- Note: was unable to reliably add invalid permissions check test that would work locally
Any manual testing needed?
- was also tested manually and will be QA'd

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

+  const [recordPlugin, setRecordPlugin] = useState<RecordPlugin | null>(null)
+  // to eventually show the user the available audio devices
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const [availableAudioDevices, setAvailableAudioDevices] = useState<


raethlein · 2024-09-06T11:43:15Z

+        *,  # keyword-only arguments:
+        disabled: bool = False,
+        label_visibility: LabelVisibility = "visible",
+    ) -> UploadedFile | None:


we unfortunately do not handle this consistently across widgets, but I want to highlight that using a Streamlit-defined type might not be the best UX because the developer sees the type, but has to click on it to get what it really means and we discourage developers to import it from here because we don't give any guarantees about our internal package structure.
I believe we should move all of our type definitions into a module streamlit.types or similar and make that an official part of our API (guarantee the existence) at some point. Until then, it might be nicer to duplicate the definition for the publicly exposed API here and write Union[UploadedFile, DeletedFile, None] (although also here two Streamlit-types are part of, so probably not worth it 😅)

No immediate action item for this PR, but wanted to share one inconsistency in our codebase right now so that we can all think about it 🙂

@raethlein I definitely agree that types as part of Streamlit API is a great idea!
We need to think carefully about exact types (especially for file-uploader, data-editor like widgets). But on the strategic level, it is something we should do at some point!

lukasmasuch

Great implementation & feature 🚀 it looks almost ready for me, I have a couple comments on testing and some observations here:

The responsiveness of the widget seems to be quite good. The case where it doesn't work well is when it is missing the microphone permission (maybe the StyledWaveformContainerDiv needs an overflow: hidden?):

Another observation: clicking on start recording - if there is already an old recording - triggers a rerun. This rerun seems to be a bit unnecessary. Is this expected?

Screen.Recording.2024-09-13.at.22.49.03.mov

A strange observation: changing the theme via the menu seems to consistently cause some trouble with the placing of the waves:

sfc-gh-nbellante · 2024-09-13T23:35:14Z

Another observation: clicking on start recording - if there is already an old recording - triggers a rerun. This rerun seems to be a bit unnecessary. Is this expected?

@lukasmasuch the reason the rerun is happening is because its deleting the old file before recording the new. perhaps we should not do that to avoid this rerun?

lukasmasuch · 2024-09-14T00:36:07Z

the reason the rerun is happening is because its deleting the old file before recording the new. perhaps we should not do that to avoid this rerun?

Hmm, yeah it looks like we don't need to delete the file / rerun up until the point where the new recording is available (on stop recording)? Two potential options:

on start recording: delete file but don't call setFileUploaderStateValue
or delete the old recording file only on stopRecording before uploading the new data (maybe the safer option)

Clicking the clear button might be the only case where we want to delete the file & setFileUploaderStateValue to empty.

sfc-gh-nbellante · 2024-09-16T20:09:45Z

or delete the old recording file only on stopRecording before uploading the new data (maybe the safer option)

i've switched it to follow this suggestion

Accidental approval

…ing)

…entState hook

## Describe your changes Rename `st.audio_input` to `st.experimental_audio_input` ## GitHub Issue Link (if applicable) ## Testing Plan - Explanation of why no additional tests are needed - Unit Tests (JS and/or Python) - E2E Tests - Any manual testing needed? --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

sfc-gh-kmcgrady

Protobuf is backwards-compatible

sfc-gh-braethlein · 2024-09-25T17:19:22Z

+message AudioInput {
+  string id = 1;
+  string label = 2;
+  string help = 3;
+  string form_id = 4;
+  bool disabled = 5;
+  LabelVisibilityMessage label_visibility = 6;


I think this is another aspect we should codify: whether or not to use optional I think we use it pretty inconsistently 😅

## Describe your changes 🔉 This PR introduces the `st.experimental_audio_input` widget! 🎤 ### Light Theme Demo ![Kapture 2024-09-13 at 11 44 38](https://github.com/user-attachments/assets/56530c43-31f4-498c-b322-df52f6be804a) ### Dark Theme Demo ![Kapture 2024-09-13 at 11 48 59](https://github.com/user-attachments/assets/70069859-bc58-4eb8-ad7c-8aff6bdaf74f) ### Light Theme No Mic Permissions <img width="755" alt="image" src="https://github.com/user-attachments/assets/51df40c7-1c75-415b-bc70-59fdb59f1c3b"> ### Dark Theme No Mic Permissions <img width="752" alt="image" src="https://github.com/user-attachments/assets/8f8d47f6-2195-4dfc-ab41-07231ae3a15b"> Note: this currently does not work on safari and by extension iOS so users on that platform will see a warning with the audio_input disabled ![image](https://github.com/user-attachments/assets/0b882904-3f66-464e-9a9b-3587805a33a9) ## GitHub Issue Link (if applicable) streamlit#8899 streamlit#903 ## Testing Plan - Unit Tests (JS and/or Python) - python unit tests added ✅ - E2E Tests - e2e tests added ✅ - Note: was unable to reliably add invalid permissions check test that would work locally - Any manual testing needed? - was also tested manually and will be QA'd --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. --------- Co-authored-by: Karen Javadyan <kajarenc@gmail.com> Co-authored-by: Bob Nisco <bob.nisco@snowflake.com>

sfc-gh-nbellante added security-assessment-completed impact:users PR changes affect end users change:feature PR contains new feature or enhancement implementation labels Sep 5, 2024

github-advanced-security AI found potential problems Sep 5, 2024

View reviewed changes

sfc-gh-nbellante marked this pull request as ready for review September 5, 2024 19:19

sfc-gh-nbellante requested a review from a team as a code owner September 5, 2024 19:19

sfc-gh-bnisco reviewed Sep 5, 2024

View reviewed changes

github-advanced-security AI found potential problems Sep 5, 2024

View reviewed changes

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInput.tsx Fixed

raethlein reviewed Sep 6, 2024

View reviewed changes

sfc-gh-nbellante mentioned this pull request Sep 6, 2024

[WIP] audio input widget prototype #9166

Closed

lukasmasuch reviewed Sep 6, 2024

View reviewed changes

lukasmasuch reviewed Sep 7, 2024

View reviewed changes

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInput.tsx Outdated

github-advanced-security AI found potential problems Sep 10, 2024

View reviewed changes

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInputActionButton.tsx Fixed

sfc-gh-nbellante force-pushed the nico/audio-input branch from d0936b0 to 763dc23 Compare September 10, 2024 22:50

raethlein mentioned this pull request Sep 12, 2024

[Feature] Add new st.pills widget #9283

Merged

sfc-gh-nbellante force-pushed the nico/audio-input branch from d7b5d0c to e51d334 Compare September 13, 2024 18:28

lukasmasuch reviewed Sep 13, 2024

View reviewed changes

lukasmasuch reviewed Sep 16, 2024

View reviewed changes

Comment thread ...ght/__snapshots__/linux/st_audio_input_test/st_audio_input_default[light_theme-chromium].png Outdated

lukasmasuch reviewed Sep 16, 2024

View reviewed changes

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInput.tsx

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInput.tsx

sfc-gh-nbellante force-pushed the nico/audio-input branch from d650c77 to b52963b Compare September 17, 2024 21:05

lukasmasuch previously approved these changes Sep 18, 2024

View reviewed changes

Comment thread ...ht/__snapshots__/linux/st_audio_input_test/st_audio_input-disabled[light_theme-chromium].png

lukasmasuch reviewed Sep 18, 2024

View reviewed changes

Comment thread frontend/lib/src/components/widgets/AudioInput/AudioInput.tsx Outdated

lukasmasuch reviewed Sep 18, 2024

View reviewed changes

Comment thread __snapshots__/darwin/st_audio_input_test/st_audio_input-default[dark_theme-chromium].png Outdated

lukasmasuch reviewed Sep 18, 2024

View reviewed changes

Comment thread frontend/lib/package.json Outdated

sfc-gh-nbellante force-pushed the nico/audio-input branch from 3274b81 to 1547acb Compare September 18, 2024 19:59

lukasmasuch reviewed Sep 18, 2024

View reviewed changes

Comment thread e2e_playwright/st_audio_input.py Outdated

sfc-gh-nbellante and others added 18 commits September 24, 2024 13:26

updating disabled screenshots

9ba5b12

removing snapshots

7cd46b6

adding comments to e2e tests

22c8b09

cleaning up streamlit test code

9168372

removing console logs and fixing equality check

a4d3122

updating snapshots?

4ea270f

break the tests (or rather make them fail because they should be fail…

7d54b83

…ing)

make it retain state after unmount

7e2aaf7

fix hover states

34c9f63

linttttttttttttttt

d66bf40

refactor: update audio input tests and implement useWidgetManagerElem…

35c684f

…entState hook

only request the mic perm when the user starts to record

4e0fc61

updating snapshots

bee8188

cleanup

1a586a8

updating tests

6f54d36

adding comment to useWidgetManagerElementState

d1e018f

delete this file

f82ca36

sfc-gh-nbellante force-pushed the nico/audio-input branch from d444bf9 to 71c6c6e Compare September 24, 2024 20:26

sfc-gh-nbellante added 5 commits September 24, 2024 14:09

adding safari blockade

f5f9d74

lint

0d1f2b2

updating test and snapshots

183da6c

couple more snapshot updates

838bfd2

updates based on feedback + some more tests

4360845

sfc-gh-nbellante changed the title ~~st.audio_input~~ st.experimental_audio_input Sep 24, 2024

sfc-gh-kmcgrady approved these changes Sep 25, 2024

View reviewed changes

sfc-gh-braethlein approved these changes Sep 25, 2024

View reviewed changes

sfc-gh-nbellante merged commit 7d70195 into develop Sep 25, 2024

sfc-gh-nbellante deleted the nico/audio-input branch September 25, 2024 19:59

Conversation

sfc-gh-nbellante commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Light Theme Demo

Dark Theme Demo

Light Theme No Mic Permissions

Dark Theme No Mic Permissions

GitHub Issue Link (if applicable)

Testing Plan

Uh oh!

Check notice

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

raethlein Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

kajarenc Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukasmasuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sfc-gh-nbellante commented Sep 13, 2024

Uh oh!

lukasmasuch commented Sep 14, 2024

Uh oh!

Uh oh!

sfc-gh-nbellante commented Sep 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sfc-gh-kmcgrady left a comment

Choose a reason for hiding this comment

Uh oh!

sfc-gh-braethlein Sep 25, 2024

sfc-gh-nbellante commented Sep 5, 2024 •

edited

Loading

sfc-gh-nbellante commented Sep 16, 2024 •

edited

Loading