Skip to content

Question and possible issue - initialising transcript component #113

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

Closed
ivanji opened this issue Mar 14, 2019 · 5 comments
Closed

Question and possible issue - initialising transcript component #113

ivanji opened this issue Mar 14, 2019 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@ivanji
Copy link

ivanji commented Mar 14, 2019

Hi guys, so I've been experiencing an issue with local transcripts and hope you could help me understand a bit more what could be happening and if my approach is correct.

My previously edited local transcripts weren't loading, (due to the fact my mediaUrls contains a session signature appended to it which change periodically). After searching for localStorage load/save methods and updating this.props.mediaUrl to this.props.fileName, things still didn't work, unless I also update the componentDidUpdate lifecyle under TranscriptEditor.

Once I also updated prevProps.mediaUrl within this method my local transcripts started loading successfully. The problem then is that every time I started editing a transcript, the cursor started jumping to the beginning of the editor, and I could see in the console these repeated messages:

Transcript first and then media 
was already present in local storage...  

as the method keeps updating/executing it seems to be checking if the transcript is available locally - so I'm not understanding the purpose of why this code is under the componentDidUpdate method (or purpose of it, and the repetition under different conditions).

componentDidUpdate(prevProps, prevState) {
    // Transcript and media passed to component at same time
    if (
      (prevState.transcriptData !== this.state.transcriptData)
        && (prevProps.mediaUrl !== this.props.fileName )
    ) {
      console.info('Transcript and media');
      this.ifPresentRetrieveTranscriptFromLocalStorage();
    }
    // Transcript first and then media passed to component
    else if (
      (prevState.transcriptData === this.state.transcriptData)
      && (prevProps.mediaUrl !== this.props.mediaUrl)
    ) {
      console.info('Transcript first and then media');
      this.ifPresentRetrieveTranscriptFromLocalStorage();
    }
    // Media first and then transcript passed to component
    else if (
      (prevState.transcriptData !== this.state.transcriptData)
      && (prevProps.mediaUrl === this.props.fileName)
    ) {
      console.info('Media first and then transcript');
      this.ifPresentRetrieveTranscriptFromLocalStorage();
    }
  }

don't know if these same changes (from mediaUrl -> fileName) caused an unexpected behaviour, but having this check running on every single update doesn't seem right.

I've already applied a solution to the issue by running the isPresentInLocalStorage method once during the componentDidMount lifecyle method instead, and I can edit my transcripts without hiccups. But I'm not sure if I'm missing something that you guys had in mind with the above code.

Don't know if this could be a section we could improve - and if there's scope also to provide an alternative prop to be used as title for localStorage as well.

Thanks!

@ivanji ivanji added the bug Something isn't working label Mar 14, 2019
@pietrop
Copy link
Contributor

pietrop commented Mar 14, 2019

I think this was addressed in PR #72

It was trying to address these scenarios:

  • if you give both media (audio or video) and transcript json to the component at the same time, then all is good

But

  • if you give first media and then after some time the transcript json
  • or if you give first the transcript json and then the media

Then this was causing problems, in having enough info to know whether to reliably check local storage or not, within the component.

eg let's say you are fetching the json from a server so that it's passed to the component, but that you want to "reconnect" the media locally - eg as a blob. Hence the need to run this.ifPresentRetrieveTranscriptFromLocalStorage(); only when both media and transcript json are present.

If that makes sense?

There might be room to refactor this into something cleaner.


We have not come across your use case tho.

You said you have

mediaUrls contains a session signature appended to it which change periodically

Just to confirm, does this mean your mediaUrl are behind a server session and you don't have direct access to them (eg as if they were on a public S3 bucket?)

I need to double check but I think we had removed fileName and replaced it with title, as it should be possible to work out the file name from the URL end point. Altho I saw you might have re-introduced it in PR #111

@pietrop
Copy link
Contributor

pietrop commented Mar 14, 2019

also see this PR for more background #88

@pietrop
Copy link
Contributor

pietrop commented Mar 15, 2019

@ivanji see this PR helps #114

@pietrop pietrop changed the title Question and possible issue Question and possible issue - initialising transcript component Mar 18, 2019
@ivanji
Copy link
Author

ivanji commented Mar 19, 2019

Thanks for this @pietrop - I'll do some testing and report my findings.

@pietrop
Copy link
Contributor

pietrop commented Mar 19, 2019

Closing this for now as I think it's addressed with last changes made to PR #114 which should fix this, but feel free to re-open or comment here if you are still having troubles with it

@pietrop pietrop closed this as completed Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants