-
Notifications
You must be signed in to change notification settings - Fork 394
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
Make YouTube Embeds GDPR compliant #3253
Conversation
* add custom youtube transformer to gatsby-remark-embedder * add needed js and css code in doc/blog components
* add on hover message to overlay
...ins/gatsby-theme-iterative-docs/src/components/Documentation/Markdown/Main/styles.module.css
Show resolved
Hide resolved
@@ -0,0 +1,16 @@ | |||
const setUpCustomYtEmbeds = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script takes all of our youtube embeds and attaches a click listener to our overlay. If the overlay is clicked, the video is played and the overlay disappears. It's definitely not very "reacty"... Does anybody have any ideas on how we could improve this?
Once we agree on a implementation (whether we deside to do this one or a different way) I want to have the code update local storage so that the message won't appear again if the user revisits the site :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does anybody have any ideas on how we could improve this?
Bit of a shot in the dark, but is it possible to have the custom embedder return a custom tag like <youtube>
and then attach a React component to our rehype-react
custom components with this logic in it? That way we can use the remark-embedder
style of automatic URL grabbing with React JS more consistent with the rest of the site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could try that but wouldn't that only work for our docs pages? We'd probably want a solution that works the same for blog and doc 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I forgot about our weird divergent blog markdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great going @julieg18!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I absolutely love the visual design on hover! Great stuff!
I think implementation could be improved a bit by my suggestion if possible, or if not at least @yathomasi's suggestion to use a hook for setUpCustomYtEmbeds
(though I'm not sure how necessary it when events are added to elements in the body as opposed to window
or document
).
@@ -0,0 +1,16 @@ | |||
const setUpCustomYtEmbeds = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does anybody have any ideas on how we could improve this?
Bit of a shot in the dark, but is it possible to have the custom embedder return a custom tag like <youtube>
and then attach a React component to our rehype-react
custom components with this logic in it? That way we can use the remark-embedder
style of automatic URL grabbing with React JS more consistent with the rest of the site.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Updated the code based on everyone's comments and implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks great.
Just a small thing I noticed on anchor links on tooltips on the different on the homepage, docs, and blogs, especially on different hover states.
HomePage - links are underlined
Docs- links are not underlined and will get underlined on hover
Blogs - links are not underlined and will get gray(opacity lower) on hover
I am not sure how much difference it makes as it is just a one-time view for users. But, if we want to be consistent I think it's good how we show on HomePage (the links are differentiated with underline)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me as well, all that's left is what @yathomasi pointed out with the overlays' links.
Thanks for spotting that! Updated all the styles, keeping them all consistent and added a focus style. |
* Make all youtube embeds gdpr compliant with on hover message * add custom youtube transformer to gatsby-remark-embedder * add needed js and css code in doc/blog components
gatsby-remark-embed
transformer and inserting a js script that handles taking off the message when the user has clicked.To Do