feat: add in-platform transcript editor to the videos page - #3215
feat: add in-platform transcript editor to the videos page#3215abhalsod-sonata wants to merge 3 commits into
Conversation
|
Thanks for the pull request, @abhalsod-sonata! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Submit a signed contributor agreement (CLA)
If you've signed an agreement in the past, you may need to re-sign. Once you've signed the CLA, please allow 1 business day for it to be processed. 🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
Hi @abhalsod-sonata! in order to get your CLA check to turn green, please have your manager reach out to oscm@axim.org to have you added to our existing entity agreement. Thank you! |
eb30fd5 to
4b28c54
Compare
CLA check resolved! |
|
@bradenmacdonald |
|
@abhalsod-sonata Thanks, this is cool! I'd definitely prefer to have the new files converted to TypeScript and ideally to Since you've expressed that it's helpful to keep the code in sync with your existing code, could you not apply the same changes to the existing codebase as well, so they stay in sync? |
Course authors can now edit video transcripts directly in Studio: "Edit transcript" in a transcript row's action menu opens an editor with per-cue text editing, SRT validation, debounced auto-save, and click-to-seek playback with active-cue highlighting against the video preview. The video's Info content moves below the preview (InfoTab), the sidebar lists transcripts directly, and the row menu label becomes "Info and transcripts" for videos.
4b28c54 to
1aa5b83
Compare
@bradenmacdonald |
|
@mphilbrick211, I’m unable to add @bradenmacdonald as a reviewer. Could you please add them to this PR? |
All set! @bradenmacdonald could you re-enable the checks to run? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3215 +/- ##
==========================================
- Coverage 95.92% 95.85% -0.08%
==========================================
Files 1397 1401 +4
Lines 33581 34067 +486
Branches 7947 8075 +128
==========================================
+ Hits 32214 32656 +442
- Misses 1308 1352 +44
Partials 59 59 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks! This is really cool. I did find a bunch of issues that we should clean up before merging, but feel free to push back on anything that doesn't make sense or is out of scope.
- It seems odd that you can edit an existing transcript but not create one from scratch. Should it be possible to create a blank transcript and then fill it out?
- When the transcript fails to download, it says "Unable to save transcript" which is a confusing error message. It should have a clear, distinct error message for this case.

- Nit/optional: The formatting of the timestamps uses a comma
,as the decimal point marker, which is incorrect for English-speaking countries and looks strange to me. I think it should use.or be based on the user's locale.

- The description says "Depends on feat: return 200 when a transcript upload replaces an existing language openedx-platform#39038", but I can't see in the diff where that would make a difference or where that return code is used.
- There are a lot more minor issues that Claude pointed out to me, which I haven't bothered to re-state here for now. I suggest you get Claude or another AI to review the PR and work through any important issues it reports and then ping me for a final review.
Thanks for the thorough review! Worked through everything - all inline comments are addressed in the latest commit; per-thread replies below. On your numbered points:
|
Yep, no problem to leave that as a future enhancement.
I do think it would be a good future enhancement. Using '.' is more natural for English speakers, and I don't think the user really cares about the detail of the SRT format. However, it's completely fine to leave as is for this PR. I'll review the other changes later; for now, I've set the tests to run again. Please make sure the test coverage checks are passing, as I can't merge otherwise. |
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks for those changes! It looks good to me now, but please check the coverage report and get the coverage checks passing, so I can merge it.
Note that if there are some things which aren't worth covering or are not possible to test properly, you can use istanbul ignore comments.
|
@bradenmacdonald - pushed a commit covering the lines Codecov flagged (add-transcript form flow, editor interactions, transcript menu, row replace/validation). The only |


Adds an in-platform transcript editor to the Studio videos page. Course authors can
open "Edit transcript" from a transcript row's action menu and edit the SRT in the
browser instead of the download → edit locally → re-upload round-trip.
What's included:
transcript-editor/module: cue-by-cue text editing with SRT parsing/validation(
srtUtils), invalid-SRT alerting, debounced auto-save, and a video preview withclick-to-seek + active-cue highlighting and auto-scroll.
InfoTabvia a newinfoModalContentUnderPreviewslot onFileTable/InfoModal), and the info sidebarlists transcripts directly - this intentionally supersedes the previous tabbed
sidebar layout for videos.
transcripts" (
FileMenu,MoreInfoColumn); two existing tests that asserted theold label on video fixtures are updated.
/transcript_upload/endpoint; the UI distinguishes replace (200) from create (201) - see dependency
below.
Impacted user roles: Course Authors (primary); Developers (new
transcript-editormodule and theinfoModalContentUnderPreviewextension point onthe generic file table).
Screen recording:
Before.mp4
After.mp4
Supporting information
/transcript_upload/) - please merge that first; without it the editor cannotdistinguish replace from create.
Testing instructions
Automated:
npm ci npx jest src/files-and-videos --coverage=false # 20 suites / 229 tests expected greenManual (tutor dev):
containing it) and run Studio.
npm run devin this repo (port 2001); open a course with videos that havetranscripts → Videos page.
the info modal shows the video preview with Info content below it and
transcripts listed in the sidebar.
reopen to confirm persistence; click cues to seek the preview.
returns 201.
Other information
PRs): rebased onto current master, which required - merging with the new
permissionsgating inFileTable/MoreInfoColumn; porting one-line changes intothe TS-renamed
FileMenu.tsxandmessages.tsfiles; re-applying theinjectIntl → useIntlmigration on top of the reworkedTranscript.jsx; andkeeping upstream's
key={idx}fix inTranscriptTab.CourseVideosTablestill passesactiveTab/setActiveTabinto the sidebar factoryfor compatibility; the new sidebar ignores them. Happy to remove that plumbing here
or in a follow-up if preferred.
seekTo()guards theplay()promise and ignoresAbortError(interrupted play on rapid cue-seek/pause/unmount), logging anything else.
Menu,AlertModal, form controls);no dedicated audit yet - feedback welcome, especially on cue-list keyboard
navigation.
descriptions; existing message ids areunchanged so downstream translations carry over.
Best Practices Checklist
.ts,.tsx).- The new
transcript-editor/module is ported as.jsx/.jsto stayreviewably close to the downstream original. Happy to convert
TranscriptEditor/srtUtilsto TS in this PR if preferred - say the word.propTypesanddefaultPropsin any new or modified code.- Ported/modified components follow the surrounding files' existing
PropTypes style; will drop them as part of the TS conversion above if requested.
src/testUtils.tsx(initializeMocks)- Changes extend the existing
videos-pagesuites, which predateinitializeMocks; kept their established setup for consistency.editor state is component-local.)
- Transcript content is fetched via the page's existing axios helpers
(
videos-page/data/api.js); this page has not been migrated to React Query yet.messagesfiles have adescriptionfor translators.../in import paths; use@srcfor parent-folder imports.Relevant backend PR