Skip to content

Fix #197 timed text editor alignement for audio files#203

Merged
emettely merged 21 commits intobbc:masterfrom
pietrop:fix-197-TimedTextEditor-alignement-for-audio-files
Oct 7, 2019
Merged

Fix #197 timed text editor alignement for audio files#203
emettely merged 21 commits intobbc:masterfrom
pietrop:fix-197-TimedTextEditor-alignement-for-audio-files

Conversation

@pietrop
Copy link
Contributor

@pietrop pietrop commented Oct 4, 2019

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

#197

Describe what the PR does

  • Allows the component to respond to the media type, if it's video goes with current display, if it's audio if goes either full width or more centred. via optional mediaType
  • Extends speaker labels to be 100% width of the container

Note that this PR branches off #201 as a base

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

Ready for review

Additional context

In current master when using an audio file with TranscriptEditor the layout looks a bit off, there is an empty column where the video preview would have been.

still to do

  • using a temporary audio file for the storybooks, needs adding a url for an audio file + corresponding transcription for storybook TranscriptEditor --> audio view.

@pietrop
Copy link
Contributor Author

pietrop commented Oct 4, 2019

considered making the component check the media type of the url to determine the layout accordingly (more info here) however this seemed to slow down the loading of the page/component coz it has to load the file before being able to determine if it's audio or video.

See below example of possible implementation in TimedTextEditor

  checkIfIsAudio = () => {
    console.log('checkIfIsAudio');
    if(this.props.mediaType === 'audio'){
      this.setState({isAudio: true})
      return true;
    }
    else{
      this.setState({isAudio: false})
      return false;
    }
 
    if (this.props.mediaUrl) {
      console.log('mediaUrl',this.props.mediaUrl)
      // from https://stackoverflow.com/questions/11876175/how-to-get-a-file-or-blob-from-an-object-url
      let file = await fetch(this.props.mediaUrl);
      const blob = await file.blob();
      console.log('blob',blob);
      console.log('type',blob.type);
      if (blob.type.includes("audio")){
        console.log('is audio')
        this.setState({isAudio: true})
        return true;
      }
      console.log('is not audio')
      this.setState({isAudio: false})
      return false;
    }
    return false;
  };

Then decided it might be better to just pass mediaType at TranscriptEditor initialization as optional, with default to video.

@pietrop
Copy link
Contributor Author

pietrop commented Oct 4, 2019

In this PR branch you can see the changes in the storybook under TranscriptEditor --> audio - http://localhost:6006/?path=/story/transcripteditor--audio-file

  • screenshots below of main views

For audio only - desktop

Screenshot 2019-10-04 at 17 43 41

Audio only - mobile

Screenshot 2019-10-04 at 17 44 05

Audio - iPad portrait

Screenshot 2019-10-04 at 17 43 52

Audio - iPad landscape

Screenshot 2019-10-04 at 17 43 57

Video - desktop

Screenshot 2019-10-04 at 17 43 13

Video - mobile

Screenshot 2019-10-04 at 17 47 30

@pietrop pietrop marked this pull request as ready for review October 4, 2019 16:55
@pietrop pietrop changed the title Fix 197 timed text editor alignement for audio files Fix #197 timed text editor alignement for audio files Oct 4, 2019
Copy link
Contributor

@emettely emettely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome changes, looks really good. I wonder if you could look for example something like the extension to determine the audio / video file as well.

Co-Authored-By: Eimi Okuno <emettely@users.noreply.github.com>
@emettely emettely merged commit ef370de into bbc:master Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants