Skip to content
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

[Voice to Content] Update waveform #20981

Merged
merged 6 commits into from
Jun 14, 2024
Merged

[Voice to Content] Update waveform #20981

merged 6 commits into from
Jun 14, 2024

Conversation

zwarm
Copy link
Contributor

@zwarm zwarm commented Jun 13, 2024

Closes https://github.com/Automattic/wordpress-mobile/issues/83

This PR updates the waveform composable

  • Use oblong indicators
  • Scroll to the left
    In addition to the waveform - the bottom sheet is scrollable on landscape

Note

The following have not yet been implemented
recording timer
orientation changes
landscape view
Overall polishing of the UI
Launch Edit Post


To Test:

  • Install the JP app from the device
  • Login and select a site that HAS AI credits
  • Navigate to Me > Debug Settings
  • Enable the voice-to-content feature flag and restart the app (option is in the more menu of the debug settings view)

Test Waveform

  • Navigate to My Site for the site AI credits
  • Tap the FAB and select the Post from audio option to launch the Post From Audio bottom sheet
  • Tap the microphone icon and begin speaking
  • ✅ Validate the waveform is visible and works as expected

Test scroll on landscape

  • Navigate to My Site for the site AI credits
  • Turn the device to landscape mode
  • Tap the FAB and select the Post from audio option to launch the Post From Audio bottom sheet
  • ✅ Validate you can scroll the view

NOTE: Orientation change will in process is not handled at the moment


Regression Notes

  1. Potential unintended areas of impact
    The waveform doesn't work and the sheet is not scrollable in landscape

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing

  3. What automated tests I added (or what prevented me from doing so)
    N/A


PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones): N/A

@zwarm zwarm requested review from pantstamp and kean and removed request for pantstamp June 13, 2024 21:05
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

Copy link

sonarcloud bot commented Jun 13, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@wpmobilebot
Copy link
Contributor

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20981-77f2344
Commit77f2344
Direct Downloadwordpress-prototype-build-pr20981-77f2344.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20981-77f2344
Commit77f2344
Direct Downloadjetpack-prototype-build-pr20981-77f2344.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 4.54545% with 63 lines in your changes missing coverage. Please review.

Project coverage is 40.75%. Comparing base (028cb1e) to head (77f2344).
Report is 11 commits behind head on trunk.

Files Patch % Lines
...ss/android/ui/voicetocontent/WaveformVisualizer.kt 0.00% 37 Missing ⚠️
.../android/ui/voicetocontent/VoiceToContentScreen.kt 0.00% 20 Missing ⚠️
.../org/wordpress/android/util/audio/AudioRecorder.kt 0.00% 5 Missing ⚠️
...droid/ui/voicetocontent/VoiceToContentViewModel.kt 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #20981      +/-   ##
==========================================
- Coverage   40.75%   40.75%   -0.01%     
==========================================
  Files        1528     1528              
  Lines       70158    70165       +7     
  Branches    11603    11607       +4     
==========================================
- Hits        28596    28595       -1     
- Misses      38972    38980       +8     
  Partials     2590     2590              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kean
Copy link

kean commented Jun 14, 2024

Design feedback:

  • When the view first appears, the "record" button starts pinned to the top of the view, then quickly jumps to the bottom
  • Animation was a bit choppy
  • The amplitude should probably be larger – it doesn't seem very responsive to the input
  • Progress view no centered

@zwarm
Copy link
Contributor Author

zwarm commented Jun 14, 2024

Design feedback:

  • When the view first appears, the "record" button starts pinned to the top of the view, then quickly jumps to the bottom
  • Animation was a bit choppy
  • The amplitude should probably be larger – it doesn't seem very responsive to the input
  • Progress view no centered

Thank you for the feedback - I'll make note of these items and address them when I polish the UI (which will be done separately after I get all the pieces working).

@zwarm zwarm merged commit 29f9541 into trunk Jun 14, 2024
21 of 22 checks passed
@zwarm zwarm deleted the issue/v2c-update-waveform branch June 14, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants