Skip to content

feat: Regions #50

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

feat: Regions #50

wants to merge 25 commits into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Jan 10, 2020 •
edited
Loading

TODO

  • "no scroll:up" support
  • clearing the display
  • final cue without a new line
  • non region cues
  • better api?

This was referenced Feb 11, 2020
Open
Open
gkatsev added 12 commits March 20, 2020 16:07
@gkatsev
keep cues and regions in the DOM and remove old cues if they aren't i…
ca2f1a9 
…n the new list
@gkatsev
add version
fa3797d
@gkatsev
check region
9e7b4a7
@gkatsev
make sure to emit pending cues, for example if there's no EOF
44c12d7
@gkatsev
move region related code to regions.js
7d067b2
@gkatsev
get scroll:up working properly
62a8123
@gkatsev
get initial positionining correct
37ef575
@gkatsev
reset region top to account for player resizing
fae8f4f
@gkatsev
properly decrease region display area when removing cues
015eb48
@gkatsev
adjust for multiple items being added to a region
683a861
@gkatsev
make batch removals slightly more reliable
db7db2d
@gkatsev
region cues should be relative to the container
d9c3f2f 
This simplifies a lot of things for region cues. It probably don't need
to keep track of box positions for them but we also don't need to do the
moveBoxToLinePosition as they will get positioned as part of the regular
flow of things.
@gkatsev gkatsev changed the title WIP: Regions feat: Regions Jun 6, 2020
@gkatsev gkatsev mentioned this pull request Jun 6, 2020
Open
2 tasks
@gkatsev
Copy link
Member Author

gkatsev commented Jun 6, 2020

This PR implements region support. It stays within the framework of the current API, which makes it a bit annoying. I think processCues probably needs a refactor for how it accepts arguments so that more changes can be made to it.
The key to making regions work is to realize that region cues should be positioned relatively and not absolutely.
There is likely some optimizations that can happen here particularly for inserting multiple cues for example during seeks.
In addition, this requires that the display area not be cleared, see videojs/video.js#6694 for usage example.

@gkatsev
Copy link
Member Author

gkatsev commented Jun 6, 2020

This still needs a couple of changes, making sure that if scroll:up isn't set, it doesn't scroll but just pops. also, verifying that regular captions are still working fine.

@gkatsev gkatsev mentioned this pull request Jul 27, 2020
Open
gkatsev added 5 commits October 7, 2020 16:06
@gkatsev
only transition when scroll:up
34b1839
@gkatsev
properly clear out the overlay
dc2228a
@gkatsev
re-use the cue for the final parsing
eb80874
@gkatsev
unneeded for regular non-region cues
5dd7293
@gkatsev
bring back styles for non-region cues.
912d291 
They were previously moved to a single block but they didn't get added
to the correct elements and thus the cues weren't positioned
appropriately.
@gkatsev
Copy link
Member Author

gkatsev commented Oct 7, 2020

A better API can wait for a complete overhaul.

@gkatsev
Copy link
Member Author

gkatsev commented Oct 7, 2020

This is a breaking change since it requires not clearing the display each tick like Video.js has been doing previously.

@gkatsev gkatsev mentioned this pull request Oct 8, 2020
Merged
Base automatically changed from master to main January 22, 2021 18:43
@littlespex littlespex mentioned this pull request May 30, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gkatsev