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

Subtitle Editor #638

Merged
merged 213 commits into from
Oct 13, 2022
Merged

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Mar 2, 2022

Includes all previous PRs for the subtitle editor.


This PR aims at integrating the different views of the subtitle editor with the parsed subtitles from Opencast.

  • The add subtitles button in the select view now has functionality
  • The subtitle select view only shows buttons for languages that already exist in Opencast or have been newly created
  • Displays error message for subtitles that could not be parsed (TODO: Make this more user friendly)

Arnei added 30 commits February 9, 2022 14:38
Any functionality of the edit view components is purely coincidental
First implementation of a list view for subtitle editing. Does not
contain any functionality yet.
- Adds a redux slice for the subtitle editor view
- Generalizes some redux selector and dispatch functions in Timeline.tsx to props. This should prepare the timeline component to work independently from the cutting view.
Need to look for  a working parser
Does NOT do anything yet, it's just UI
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions github-actions bot removed the status:conflicts Conflicts with another pull request or issue label Oct 6, 2022
@ziegenberg
Copy link
Member

Are you gonna squash those commits before merging? :)

@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This pull request is deployed at test.editor.opencast.org/638/2022-10-06_15-23-09/ .
It might take a few minutes for it to become available.

@ziegenberg ziegenberg added the type:feature A new feature or feature request label Oct 6, 2022
@Arnei
Copy link
Member Author

Arnei commented Oct 6, 2022

Are you gonna squash those commits before merging? :)

Not planning to. While it's a hell of a lot of commits, most of them actually do relevant stuff, and squashing them all together feels like grossly disregarding the development history.

@github-actions
Copy link

github-actions bot commented Oct 7, 2022

This pull request is deployed at test.editor.opencast.org/638/2022-10-07_11-57-50/ .
It might take a few minutes for it to become available.

@ziegenberg ziegenberg linked an issue Oct 7, 2022 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Oct 7, 2022

This pull request is deployed at test.editor.opencast.org/638/2022-10-07_15-21-49/ .
It might take a few minutes for it to become available.

@lkiesow
Copy link
Member

lkiesow commented Oct 11, 2022

Sorry Arne, I found one more bug in the subtitle editor. But I hope this one is easy to fix:

Screenshot from 2022-10-11 16-56-03

HTML seems to be escaped, saved and then escaped when you load it again.

@lkiesow
Copy link
Member

lkiesow commented Oct 11, 2022

Interesting, the player also does weird things with this as input:

Screenshot from 2022-10-11 17-01-09

@Arnei
Copy link
Member Author

Arnei commented Oct 12, 2022

HTML seems to be escaped, saved and then escaped when you load it again.

Do you have more details on that? I could not reproduce that.

Bildschirmfoto vom 2022-10-12 09-42-08

Although I could get the div tags to disappear after saving and reloading

Bildschirmfoto vom 2022-10-12 09-42-44

@github-actions github-actions bot added the status:conflicts Conflicts with another pull request or issue label Oct 12, 2022
@github-actions
Copy link

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions github-actions bot removed the status:conflicts Conflicts with another pull request or issue label Oct 12, 2022
@github-actions
Copy link

This pull request is deployed at test.editor.opencast.org/638/2022-10-12_14-55-29/ .
It might take a few minutes for it to become available.

@lkiesow
Copy link
Member

lkiesow commented Oct 12, 2022

Do you have more details on that? I could not reproduce that.

opencast-editor-html.mp4

@Arnei
Copy link
Member Author

Arnei commented Oct 13, 2022

Thanks, now I can reproduce it too :)

@lkiesow
Copy link
Member

lkiesow commented Oct 13, 2022

As discussed in Matrix, I'm pretty happy with the current state of the new subtitle editor. The minor problem with the potential double-encoding of HTML content in subtitles, which is a somewhat obscure problem normal users are unlikely to face I'm going to ignore here.
Given that this is still merged as beta, and hidden by default, and that we have a few suggestions for further improvements for the future already, I think that's okay,
Iignoring that and merging it like this so that users can already start tesing that (and probably find more issues :D) probably makes more sense instead of waiting for just the next merge conflict. We will spend some more time on this anyway. Of course, I will create an issue for that.

@ziegenberg ziegenberg linked an issue Oct 13, 2022 that may be closed by this pull request
@lkiesow lkiesow merged commit 1a63c95 into opencast:main Oct 13, 2022
@lkiesow lkiesow changed the title SubtitleEditor Subtitle Editor Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature A new feature or feature request view:subtitle-editor
Projects
None yet
5 participants