Skip to content

Fix 87 initialising transcript media - component interface #88

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

Merged
merged 6 commits into from
Jan 23, 2019

Conversation

pietrop
Copy link
Contributor

@pietrop pietrop commented Jan 23, 2019

Is your Pull Request request related to another issue in this repository ?
If so please link to other issues and PRs as appropriate

#87

Describe what the PR does
A clear and concise description of what the PR does. Feel free to use bulletpoints and checkboxes if needed [...]

As explained in the QA section

The component at a minimum needs two params to function properly and to be able to save to local storage. Transcript as transcriptData (+ sttJsonType) and media asmediaUrl.

We'd normally expect both to be provided at the same time, but there might be edge case where the component is initialized with the first one, and then subsequently receives the second one (or the other way around).

This PR addresses this issue, making it possible to initialise the component with either both, or with one or the other first subsequently followed by the other.

State whether the PR is ready for review or whether it needs extra work
If you are still working on it and just setting it up for later review, or if it's ready to be reviewed for merging

almost ready for review

Additional context
Add any other context or screenshots about the PR.

NA

@pietrop
Copy link
Contributor Author

pietrop commented Jan 23, 2019

QA testing notes

✅ Both at same time - media+transcript

Steps:
  • click 'clear local storage'
  • 'Load demo'
  • Edit text
  • refresh browser
  • 'Load demo'
Expected Results:
  • Expect the text change in step 3 to have persisted

✅ Media first - local media

Steps:
  • click 'clear local storage'
  • 'Load Local Media' + 'Chose File'
  • 'open Transcript Json' + 'Choose file'
  • Edit text
  • refresh browser
  • repeat step 2 and 3
Expected Results:
  • Expect the text change in step 4 to have persisted

✅ Media first - url

Steps:
Expected Results:
  • Expect the text change in step 4 to have persisted

✅ Transcript first - local media

Steps:
  • click 'clear local storage'
  • 'open Transcript Json' + 'Choose file'
  • 'Load Local Media' + 'Chose File'
  • Edit text
  • refresh browser
  • repeat step 2 and 3
Expected Results:
  • Expect the text change in step 4 to have persisted

✅ Transcript first - url

Steps:
Expected Results:
  • Expect the text change in step 4 to have persisted

@pietrop pietrop merged commit 56bed8d into master Jan 23, 2019
@pietrop pietrop deleted the fix-87-initialising-transcript-media branch January 23, 2019 23:05
@pietrop pietrop restored the fix-87-initialising-transcript-media branch March 15, 2019 12:25
@jamesdools jamesdools deleted the fix-87-initialising-transcript-media branch March 15, 2019 14:41
@pietrop pietrop restored the fix-87-initialising-transcript-media branch March 19, 2019 13:31
@pietrop pietrop deleted the fix-87-initialising-transcript-media branch March 20, 2019 18:58
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.

1 participant