Fix #5627 : Stop recording should stop microphone and angular-recorder should be replaced. #6708

mzaman07 · 2019-05-07T19:31:40Z

Explanation

Fixes #5627 by stopping the microphone stream when user stops recording. Fixes #6388 by replacing the angular-recorder library. Design doc provides more background info on changes and the reasoning for choosing this method.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
The PR is made from a branch that's not called "develop".
The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).
The PR follows the style guide.
The PR addresses the points mentioned in the codeowner checks for the files/folders changed. (See the codeowner's wiki page.)
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

…Modified html and js in AudioTranslationBarDirective.

…ab while recording.

… for mp3converter worker.

mzaman07 · 2019-05-07T19:42:49Z

Looking for feedback on design, code for the rough setup I have.

So this is still a WIP, there is an issue when you click on stateTranslation when recording.
Steps to reproduce.

Set $rootScope.loadingMessage = 'loading' in line 406 in AudioTranslationBarDirective.ts
Create a full exploration that is capable of having multiple translations.
Start recording
Click on different tab on StateTranslation during the recording.

The result of this will be the page will be stuck in "loading..."

When $rootScope.loadingMessage = '' it works fine but based on running debugger for initAudioBar it shows the message as blank/ not obviously visible.

.../templates/dev/head/pages/exploration_editor/translation_tab/AudioTranslationBarDirective.ts

mzaman07 · 2019-05-09T21:09:00Z

Once I figure out how to set a web-worker script to play nicely with the typescript compiler, I think those tests will pass. I will also write a test for it as well.

mzaman07 · 2019-05-13T21:00:25Z

Is it okay that I include https://github.com/microsoft/TypeScript/blob/master/lib/lib.webworker.d.ts as part of the typings. I need it so it can compile to ts, if I need to cut this down I will do that.

seanlip · 2019-05-13T21:58:50Z

/cc @ankita240796 (who is our authority on TypeScript!)

ankita240796 · 2019-05-13T22:05:20Z

Is it okay that I include https://github.com/microsoft/TypeScript/blob/master/lib/lib.webworker.d.ts as part of the typings. I need it so it can compile to ts, if I need to cut this down I will do that.

Hi @mzaman07, can it be not added in typings present in node_modules folder, why do we need it in custom typings?

mzaman07 · 2019-05-13T22:12:20Z

I see it is already there, ok I'll just use that instead. The reason for adding was that I wasn't sure if it was already in the codebase.

nithusha21 · 2019-05-14T17:28:38Z

Hi @mzaman07 please note the failing tests.

oppiabot · 2019-05-14T17:31:21Z

Hi @mzaman07. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

DubeySandeep · 2019-05-27T20:43:40Z

@YashJipkate, Can you please review this PR? (As discussed in the meeting!)

apb7

LGTM from code owner's perspective. Thanks, @mzaman07!

package.json

ankita240796

LGTM from code owner's perspective, Thanks @mzaman07!

core/templates/dev/head/AppInit.ts

package-lock.json

vojtechjelinek · 2019-05-29T18:53:09Z

@mzaman07 Please, also add "Fixes #6388" to the description so it is closed automatically.

seanlip

LGTM as codeowner, thanks @mzaman07!

nithusha21

LGTM from a codeowner's perspective.

DubeySandeep

@mzaman07, Can you please update your PR with upstream, I'm unable to merge this PR as the codecov tests are not running. Also, I've left a few comments (simple nit) related to code comment It would be great if we can fix those and merge this PR quickly! :)

/cc @oppia/dev-workflow-team

.../templates/dev/head/pages/exploration_editor/translation_tab/AudioTranslationBarDirective.ts

…r naming convention.

mzaman07 · 2019-06-01T03:43:00Z

Dunno why the codecov is not working. I'll write tests going through all of Voiceover component then. It could be it's own test script or expand on explorationTranslationTab.js .

Edit: Looks like I'll need touch these settings to make tests work. For now I'll just push without tests.

lilithxxx · 2019-06-01T05:45:20Z

Ok I guess I see a pattern. @mzaman07 are you using your own instance of circleci? This is creating problems with the codecov tests.

/cc @oppia/dev-workflow-team

apb7 · 2019-06-01T09:23:20Z

Hi @mzaman07, can you please temporarily unfollow mzaman07/oppia by visiting https://circleci.com/gh/organizations/mzaman07/settings#projects? Once you've done that, update your branch to re-trigger the builds at oppia/oppia. This will un-stall codecov. Thanks!

mzaman07 · 2019-06-01T15:32:44Z

Oh I see ok thanks. Yeah I was using my own to test something awhile back.

mzaman07 added 6 commits May 6, 2019 23:42

added base functional recording service to replace angular-recorder. …

6bc598c

…Modified html and js in AudioTranslationBarDirective.

Something wrong with rootScope loadingMessage when you change state t…

e7210a0

…ab while recording.

lint fixed

c85f282

webworker compilation in ts set

3cf277c

reverted tsconfig.json and added karma framework thread loading check…

69881bc

… for mp3converter worker.

lint fixes

d7f0848

mzaman07 added frontend audio-translation CHANGELOG: Bug fixes labels May 7, 2019

mzaman07 requested a review from DubeySandeep May 7, 2019 19:31

mzaman07 requested review from seanlip and vojtechjelinek as code owners May 7, 2019 19:31

DubeySandeep reviewed May 7, 2019

View reviewed changes

.../templates/dev/head/pages/exploration_editor/translation_tab/AudioTranslationBarDirective.ts Outdated Show resolved Hide resolved

DubeySandeep removed CHANGELOG: Bug fixes labels May 8, 2019

nithusha21 assigned mzaman07 May 14, 2019

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label May 14, 2019

mzaman07 added 2 commits May 16, 2019 21:20

updated branch and fixed merge conflicts.

c086c4f

Changed mp3Conveter to a ts-compiler friendly version.

874ca92

vojtechjelinek assigned seanlip, nithusha21, ankita240796, DubeySandeep and apb7 and unassigned DubeySandeep May 27, 2019

DubeySandeep assigned YashJipkate May 27, 2019

apb7 approved these changes May 28, 2019

View reviewed changes

package.json Show resolved Hide resolved

apb7 removed their assignment May 28, 2019

ankita240796 approved these changes May 28, 2019

View reviewed changes

YashJipkate reviewed May 29, 2019

View reviewed changes

core/templates/dev/head/AppInit.ts Outdated Show resolved Hide resolved

package-lock.json Show resolved Hide resolved

seanlip approved these changes May 29, 2019

View reviewed changes

nithusha21 approved these changes May 29, 2019

View reviewed changes

nithusha21 unassigned nithusha21, seanlip and ankita240796 May 29, 2019

mzaman07 added 2 commits May 29, 2019 21:27

Merge remote-tracking branch 'upstream/develop' into mp3-recorder

cf8566f

changed AppInit.ts for improved readability.

1b269cd

DubeySandeep approved these changes May 31, 2019

View reviewed changes

DubeySandeep reviewed May 31, 2019

View reviewed changes

mzaman07 added 2 commits May 31, 2019 19:33

Merge remote-tracking branch 'upstream/develop' into mp3-recorder

2ff1664

Changed comments to be a full sentence and switched variable to prope…

a5ebd6c

…r naming convention.

Corrected comment because action is singular as opposed to plural.

49bf1b7

DubeySandeep merged commit 5fad146 into oppia:develop Jun 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #5627 : Stop recording should stop microphone and angular-recorder should be replaced. #6708

Fix #5627 : Stop recording should stop microphone and angular-recorder should be replaced. #6708

mzaman07 commented May 7, 2019 •

edited

Loading

mzaman07 commented May 7, 2019 •

edited

Loading

mzaman07 commented May 9, 2019 •

edited

Loading

mzaman07 commented May 13, 2019 •

edited

Loading

seanlip commented May 13, 2019

ankita240796 commented May 13, 2019

mzaman07 commented May 13, 2019 •

edited

Loading

nithusha21 commented May 14, 2019

oppiabot bot commented May 14, 2019

DubeySandeep commented May 27, 2019

apb7 left a comment

ankita240796 left a comment

vojtechjelinek commented May 29, 2019 •

edited

Loading

seanlip left a comment

nithusha21 left a comment

DubeySandeep left a comment

mzaman07 commented Jun 1, 2019 •

edited

Loading

lilithxxx commented Jun 1, 2019

apb7 commented Jun 1, 2019

mzaman07 commented Jun 1, 2019 •

edited

Loading

Fix #5627 : Stop recording should stop microphone and angular-recorder should be replaced. #6708

Fix #5627 : Stop recording should stop microphone and angular-recorder should be replaced. #6708

Conversation

mzaman07 commented May 7, 2019 • edited Loading

Explanation

Checklist

mzaman07 commented May 7, 2019 • edited Loading

mzaman07 commented May 9, 2019 • edited Loading

mzaman07 commented May 13, 2019 • edited Loading

seanlip commented May 13, 2019

ankita240796 commented May 13, 2019

mzaman07 commented May 13, 2019 • edited Loading

nithusha21 commented May 14, 2019

oppiabot bot commented May 14, 2019

DubeySandeep commented May 27, 2019

apb7 left a comment

Choose a reason for hiding this comment

ankita240796 left a comment

Choose a reason for hiding this comment

vojtechjelinek commented May 29, 2019 • edited Loading

seanlip left a comment

Choose a reason for hiding this comment

nithusha21 left a comment

Choose a reason for hiding this comment

DubeySandeep left a comment

Choose a reason for hiding this comment

mzaman07 commented Jun 1, 2019 • edited Loading

lilithxxx commented Jun 1, 2019

apb7 commented Jun 1, 2019

mzaman07 commented Jun 1, 2019 • edited Loading

mzaman07 commented May 7, 2019 •

edited

Loading

mzaman07 commented May 7, 2019 •

edited

Loading

mzaman07 commented May 9, 2019 •

edited

Loading

mzaman07 commented May 13, 2019 •

edited

Loading

mzaman07 commented May 13, 2019 •

edited

Loading

vojtechjelinek commented May 29, 2019 •

edited

Loading

mzaman07 commented Jun 1, 2019 •

edited

Loading

mzaman07 commented Jun 1, 2019 •

edited

Loading