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

Added @vjs-font-path variable #1847

Closed
wants to merge 1 commit into from
Closed

Conversation

iSimonWeb
Copy link
Contributor

It's a simple PR.
I just need to be able to change the font path since I'm using video-js via bower and the "font" directory of my project is not a subpath of "css".

Awesome Vanilla plugin anyway! Thank you! 👍

It's a simple PR.
I just need to be able to change the font path since I'm using video-js via bower and the "font" directory of my project is not a subpath of "css".

Awesome Vanilla plugin anyway! Thank you!
@heff
Copy link
Member

heff commented Feb 11, 2015

Thanks for the help! So does this allow you to override the value of vjs-font-path then? Is there potentially a cleaner way we should be handling this for people who are including this via bower?

@mmcc thoughts?

@mmcc
Copy link
Member

mmcc commented Feb 11, 2015

@heff No, this doesn't allow you to override the value of vjs-font-path. As far as I know, this really just cuts down the number of places you need to type your new font-path (which is helpful).

I'm fine with pulling this in, but it's going to be fairly short lived.

@iSimonWeb
Copy link
Contributor Author

Thank you guys for the attention.
I've seen this approach in the Font-Awesome project
Here is the variable -> https://github.com/FortAwesome/Font-Awesome/blob/master/less/variables.less
And there is where are they using it -> https://github.com/FortAwesome/Font-Awesome/blob/master/less/path.less

Hope you'll appreciate 👍

@heff heff closed this in 3459381 Feb 12, 2015
@heff
Copy link
Member

heff commented Feb 12, 2015

Thanks @iSimonWeb!

@iSimonWeb iSimonWeb deleted the patch-1 branch August 27, 2017 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants