Fix 87 initialising transcript media v2

[pietrop]

Is your Pull Request request related to another issue in this repository ?

Recently noticed in PR #111 but also present in current master and flagged in this issue #113

Originally logged as this issue #87 and addressed as this PR #88

Describe what the PR does

When initialising the TranscriptEditor component you might have

1. Both media url (audio or video) and transcript json present
2. Media url is present and transcript json is added later
3. Media url is added later and transcript json is present at initialisation

And it also needs to work, when saving to local storage and retrieving data from it.

In PR #88 fix all these scenarios but in current master 3 is not working.

This PR tries to figure out the differences between current master and previously fixed PR #88

State whether the PR is ready for review or whether it needs extra work

Not ready for merge, just using the diff to see what's the difference with current master.

Additional context

The idea is to rebase, and re-visit once these PRs have been added to master.

• PR Layout changes accessibility #111
• PR Stt adapter awstranscribe #110

[pietrop]

in src/lib/TranscriptEditor/index.js - function ifPresentRetrieveTranscriptFromLocalStorage

  ifPresentRetrieveTranscriptFromLocalStorage = () => {
-  if (this.refs.timedTextEditor!== undefined) {
-      if (this.refs.timedTextEditor.isPresentInLocalStorage(this.props.mediaUrl)) {
-        console.log('was already present in local storage');
-        this.refs.timedTextEditor.loadLocalSavedData(this.props.mediaUrl);
+    if (this.timedTextEditorRef.current!== undefined) {
+      if (this.timedTextEditorRef.current.isPresentInLocalStorage(this.props.mediaUrl)) {
+        console.info('was already present in local storage');
+        this.timedTextEditorRef.current.loadLocalSavedData(this.props.mediaUrl);
      } else {
        console.info('not present in local storage');
      }
    }
  }

in red how it was (when it was working), in green how it is current PR (not working)

seems like it can be simplified as

 ifPresentRetrieveTranscriptFromLocalStorage = () => {
    if (this.timedTextEditorRef!== undefined) {
        if (this.timedTextEditorRef.isPresentInLocalStorage(this.props.mediaUrl)) {
            console.info('was already present in local storage');
            this.timedTextEditorRef.loadLocalSavedData(this.props.mediaUrl);
        } else {
            console.info('not present in local storage');
        }
    }
}

[jamesdools]

Remove the console.logs and it's all good! 👍

[pietrop]

Not ready for merge

Just done further testing, doesn't work in conjunction with local storage.

See QA docs docs/qa/0-component-interface.md for more info on how to test.

[pietrop]

current issue, when initialising component in this branch:

• video and transcript at same time --> works with local storage
• video first then transcript --> doesn't work with local storage
• transcript first then video --> crashes

crash error TypeError: Cannot read property 'isPresentInLocalStorage' of null

src/lib/TranscriptEditor/index.js:83

 if (this.timedTextEditorRef.current.isPresentInLocalStorage(this.props.mediaUrl)) {

[pietrop]

Ready for review/Merge

Digged out old PRs and Issues addressing this problem and fixed it.

Initial issue #71 and PR #72

TranscriptEditor component can be initialised either with url (1)or from local media (blob) (2) and it would restore session from local storage if previously saved.

When adding from local media, TranscriptEditor can take an optional fileName attribute (as explained in #72 - #71) which creates a unique key to save to local storage.

See demo app in src/index.js line 36 for example of how to read file name in the browser client side to initialise the component with it.

 // https://stackoverflow.com/questions/8885701/play-local-hard-drive-video-file-with-html5-video-tag
  handleChangeLoadMedia(files) {
    console.log(files);
    const file = files[0];
    const type = file.type;
    // check if is playable
    const videoNode = document.createElement('video');
    const canPlay = videoNode.canPlayType(type);
    if (canPlay) {
      const fileURL = URL.createObjectURL(file);
      // videoNode.src = fileURL
      this.setState({
        // transcriptData: kaldiTedTalkTranscript,
        mediaUrl: fileURL,
        fileName: file.name
      });
    }
    else {
      alert('select a valid audio or video file');
    }
  }

When testing (1) and (2) note that even if we use it's "the same file", those are considered two different inputs by the component, as the component uses file name string for local media and the url uses the url string input to more uniquely identify transcriptions.

Recognising whether is the same file, across url input string (1), and local file (2), it's outside of scope. Supporting local media is an edge case, we'd assume, and recommend most often the component be initialised with both media url and and transcript data (json) at the same time.

QA doc -0-component-interface.md can be used forr more comprehensive test of all the different scenarios.

It might also be good to clean up this notes and add them the docs or make an ADR.

[pietrop]

TL;DR:

to recap at high level

When initialising the component there might be a race condition between media url + transcript Json content if not provided at same time

This can cause problem when retrieving from local storage

complicating things even further, if the media url comes from local media (and therefore as blob url ) then then url string is not a unique identifier that can be used to save and retrieve from local storage.

as a workaround added an optional file name to use local storage with local media