Skip to content
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

New Editor Interface #297

Merged
merged 11 commits into from
Aug 8, 2024
Merged

Conversation

kouloumos
Copy link
Member

@kouloumos kouloumos commented Jun 23, 2024

Overview

This PR closes #280, it introduces a new Editor interface built using our own modified version of the slate-transcript-editor. Additionally, it includes a significant refactoring of the related code to streamline logic and eliminate redundant code from previous iterations.

I have made an effort to break down the changes into granular commits to facilitate the review process. However, the commit 1d36b53 could not be further divided. I hope the detailed commits elsewhere will assist reviewers in their evaluation.

Details

The new Editor introduces a new data format (DPE), stored for each transcript in the bitcointranscripts-metadata repository and used as input for the slate-transcript-editor. When a user claims a transcript, we now create two branches: one for DPE and one for markdown. Similarly, when they submit, we open two Pull Requests, one for each repository.

Testing the changes

You can test the changes by pointing your local instance to staging with the following .env variables

NEXT_PUBLIC_APP_TRANSCRIPTION_BASE_URL = "https://brilliant-growth-staging.up.railway.app"
NEXT_PUBLIC_APP_QUEUE_BASE_URL = "https://transcription-review-backend-staging.up.railway.app/api"
NEXT_PUBLIC_VERCEL_ENV = "staging"

Then, claim a transcript and test the new Editor.

Next Steps

  • Support save upon submission.
  • Improve support for transcripts with a single speaker.
  • Improve support for "Audience" speaker attributions.

Note: the fork of slate-transcript-editor will be moved under the bitcointranscripts org at a later time.

Copy link

vercel bot commented Jun 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
transcription-review-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2024 0:33am

@0tuedon
Copy link
Contributor

0tuedon commented Jul 12, 2024

So, I did check out the UI work on the Editor, good work but I did encounter some issues

  1. Does the work with the playback feature??
Screenshot 2024-07-12 at 14 04 02 I encounter a situation where the audio didn't play.

Also took a while to get used to this editor. Also this doesn't support MD?

@kouloumos
Copy link
Member Author

I encounter a situation where the audio didn't play.

Please send a screen recording with the issue.

Also this doesn't support MD?

No. The markdown is generated upon save from the DPE file.

Copy link
Collaborator

@Emmanuel-Develops Emmanuel-Develops left a comment

Choose a reason for hiding this comment

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

Awesome features!
Here are some observations I noticed.

In-Editor Observations

  • There are situations where I'd like to change the timestamps, for example here's a snippet of a transcript
00:00:20   Aaron van Wirdum    Live from Utrecht, this is Bitcoin Explained. Hello Sjors.
00:00:25   Sjors Provoost.          What's up? Welcome back. Thank you.

and it should be this

00:00:20   Aaron van Wirdum    Live from Utrecht, this is Bitcoin Explained. Hello Sjors.
00:00:25   Sjors Provoost           What's up?
00:00:26   Aaron van Wirdum    Welcome back.
00:00:27   Sjors Provoost           Thank you.

There's no way to edit that on the editor for now. In this case it's a minor issue, but there's no way to resolve it than creating a commit outside the editor.
Apparently, using a shift+enter creates new timestamps. Attached is a screen recording.

trim.timestamp-1.mov
  • Audio Playback issues.
    Sometimes the audio refuses to play, probably the same issue @0tuedon noticed. Although if I leave it for a while it resumes.
    Could be bandwidth related (my internet speed is about 20mbps) probably redownloading everytime one skips rather than downloading to memory.

  • minor stuck state on exit speaker selection.
    If I exit the speaker selection by clicking on itself, the cancel and save buttons don't work.
    Screen recording below

click.out.480p.mov
  • Unable to create new lines: As mentioned earlier, shift+enter creates new timestamps, users are unable to create new lines in the editor. This could be useful for quotes in markdown i.e

quote here

Without new lines, some markdown tags do break.

  • submit doesn't save the last changes made to the editor.

@kouloumos
Copy link
Member Author

Regarding @Emmanuel-Develops comments:

Unable to create new lines: [...] This could be useful for quotes in markdown i.e

As I replied here markdown is not supported. The markdown file is created upon save/submission.

An onboarding flow will be added later to better explain to users what's available and how to use the interface. This includes the way to add new timestamps including everything that is currently available when you hover over the "How Does this work?" text.

submit doesn't save the last changes made to the editor.

This is a known limitation, I will include it in the description. This functionality will be added in subsequent PR.

minor stuck state on exit speaker selection.

This seems to be because of the way the dropdown is implemented. I'll see what I can do to fix it.

Copy link
Collaborator

@Extheoisah Extheoisah left a comment

Choose a reason for hiding this comment

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

Quite a long PR.
In general, the pr looks good. I left some comments and nits.
I haven't tested locally though.

necessary additions to support the library
Layout already has a `GlobalContainer`. This is the
only page that as an extra one.
- move the logic for the claim process in a single mutation hook
- refactor  `/fork` and `/newBranch` endpoints to make them more
 flexible by moving logic inside the mutation hook for `claimTranscript`
move the modal for restoring the transcript to its
original version into its own component
introduce `/github/read` used by the refactored
`useReview` hook to read all the required transcript
data directly from GitHub
removed previously used implementation of
`SelectField` and `SelectBox` components
@kouloumos
Copy link
Member Author

I updated the PR with all of @Extheoisah suggestions and I also fixed the "minor stuck state on exit speaker selection" issue pointed by @Emmanuel-Develops.

There are several changes in this commit as I couldn't break them
into more granular commits.

The changes are:
1. The new editor interface: the data that we started reading directly
from GitHub are now utilized in this new interface as part of
`SlateTranscriptEditor`. The metadata sidebar have been removed
and the required metadata fields are now part of the same page.

2. Refactoring of the save process and new PR process:
The`/save` and `/pr` endpoints were ineficiently (and with duplicate code)
handling fork and new branch scenarios alongside redundant scenarios from
previous queueing logic. Those have been simplified.
The logic for saving progress and submitting the review is now part of the
 `useGitHub` hook.

3. The `SubmitTranscriptAlert` has been moved inside the
`SubmitTranscriptModal` alongside a simplified  logic for
the submit state.
the logic for suggesting a source is now
compatible with the refactored github endpoints
and part of the `useGithub` hook
@Extheoisah Extheoisah merged commit 2aa32b1 into bitcointranscripts:dev Aug 8, 2024
2 checks passed
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.

Playback Features inside the Editor page
4 participants