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

Change style options and a new setting for intializing #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nyroDev
Copy link

@nyroDev nyroDev commented Dec 11, 2016

  • Move markerStyle to markerTip.style to be more consistent
    • This update will be a breaking change
  • Move javascript inline CSS into CSS file
  • Add marker.style option to be able to customize each marker on it's own
  • Add directInitialize setting : this is useful if you want to initialize markers later on (ex: when depending on vtt loading)

I updated the doc as far as I can see, but some elements still need to be done:

  • Update History changelog
  • Regenerate all files correctly (I did it by hand for non compressed file)

Hope this will help.

Move javascript inline CSS into CSS file
Add marker.style option
Add directInitialize setting
Copy link
Owner

@spchuang spchuang left a comment

Choose a reason for hiding this comment

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

All the changes look reasonable to me except moving/addition of markerStyle.


video.markers({
markerStyle: {
'width':'8px',
'background-color': 'red'
Copy link
Owner

Choose a reason for hiding this comment

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

I'm inclined to not make this change since it is a big breaking change to the API. Moving markerStyle to markerTip.style isn't appropriate either because markerTip is used for the hover-over pointer thing.

@spchuang
Copy link
Owner

spchuang commented May 4, 2017

@nyroDev sorry for the late reply. Just took a look and commented at the code change.

},
markers: [
{time: 9.5, text: "this"},
{time: 16, text: "is"},
{time: 16, text: "is", style: {'background-color': 'yellow'}},
Copy link
Owner

Choose a reason for hiding this comment

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

Just took another look. Currently, we offer the ability to append class to marker object like {time: 9.5, text: "this", overlayText: "1", class: "special-blue"}, which achieves the same thing as passing a style object.

Is there a scenario where you can't pass in a class name and need hardcoded style?

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