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

accurate YouTube play button #17

Closed
wants to merge 1 commit into from
Closed

accurate YouTube play button #17

wants to merge 1 commit into from

Conversation

chuanqisun
Copy link

@chuanqisun chuanqisun commented Nov 22, 2019

I saw the comment in your css /* TODO: Consider replacing this with YT's actual svg. Eh. */, so I went ahead and implemented it to address #16

Some trade-offs to consider with this change:

  1. The button will look identical to YouTube's, convincing user that this is not a phishing website
  2. The CSS file size got reduced
  3. The HTML file size increased
  4. The pro-usage snippet is longer

Feel free to reject this PR if this is not the direction you want to take this project it. It was a very quick job for me so don't worry about it.

@chuanqisun
Copy link
Author

fyi, this will cause some conflict with #8.

@paulirish
Copy link
Owner

i'd feel a bit better if we could use the SVG from CSS so the progressive enhancement story still has very clean and minimal HTML

does using the SVG as a background image present problems?

@chuanqisun
Copy link
Author

Progressive enhancement is a good point. Let me try the background image approach while resolving the conflicts.

@paulirish
Copy link
Owner

@chuanqisun thanks again for the contribution. i merged something quite similar in #39

cheers

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.

None yet

2 participants