-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] Error logging, tracking, and error state #20979
Conversation
β¦errors from audio
Generated by π« Danger |
|
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr20979-028cb1e | |
Commit | 028cb1e | |
Direct Download | jetpack-prototype-build-pr20979-028cb1e.apk |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr20979-028cb1e | |
Commit | 028cb1e | |
Direct Download | wordpress-prototype-build-pr20979-028cb1e.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #20979 +/- ##
==========================================
- Coverage 40.79% 40.75% -0.04%
==========================================
Files 1527 1528 +1
Lines 70079 70158 +79
Branches 11588 11603 +15
==========================================
+ Hits 28589 28596 +7
- Misses 38900 38972 +72
Partials 2590 2590 β View full report in Codecov by Sentry. |
@@ -25,6 +30,10 @@ class VoiceToContentUseCase @Inject constructor( | |||
file: File | |||
): VoiceToContentResult = | |||
withContext(Dispatchers.IO) { | |||
if (!networkUtilsWrapper.isNetworkAvailable()) { |
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.
Is it common on Android to stop the request without sending it in case the app thinks network is unavailable. On iOS, it's considered a bad practice because the respective API usually lags behind the real status a bit and is not always accurate.
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.
@kean - yes, this is the way it's done on Android. Something perhaps for the future app team to align?
That's odd indeed. Postman returns false for anything under 25.2. I'll see if it's set internally somewhere.
The entire layout needs to be polished. π Thanks for the feedback! |
Addresses
This PR implements the following Voice To Content
PrepareVoiceToContentUseCase
andVoiceToContentUseCase
Note
The following have not yet been implemented
waveform
recording timer
orientation changes
landscape view
Overall polishing of the UI
Launch Edit Post
To Test:
The testing will focus on tracking
voice-to-content
feature flag and restart the app (option is in the more menu of the debug settings view)Post from audio
option to launch the Post From Audio bottom sheetπ΅ Tracked: my_site_create_sheet_action_tapped, Properties: {"action":"create_new_post_from_audio"}
π΅ Tracked: voice_to_content_sheet_shown
π΅ Tracked: voice_to_content_button_recording_limit_reached
Post from audio
option to launch the Post From Audio bottom sheetπ΅ Tracked: my_site_create_sheet_action_tapped, Properties: {"action":"create_new_post_from_audio"}
π΅ Tracked: voice_to_content_sheet_shown
π΅ Tracked: voice_to_content_button_start_recording_tapped
π΅ Tracked: voice_to_content_button_done_tapped
Regression Notes
Potential unintended areas of impact
The track events are not tracked
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual tested
What automated tests I added (or what prevented me from doing so)
N/A
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.Testing Checklist (strike-out the not-applying and unnecessary ones): N/A