search

nfl / react-gpt

A React display ad component using Google Publisher Tag
MIT License
145 stars 84 forks source link

request http://www.googletagservices.com/tag/js/gpt.js #77

Closed ldfloresgon closed 6 years ago

ldfloresgon commented 6 years ago

Hello, the request "http://www.googletagservices.com/tag/js/gpt.js", can be by SSL?

Thanks

potench commented 6 years ago

The seedFile is loaded from a relative protocol https://github.com/nfl/react-gpt/blob/8278236156b11ba35619bede3e32219d4c24f41b/src/Bling.js#L239 I suppose we could force it to be https or allow an override, or you can host your site securely.

ldfloresgon commented 6 years ago

thanks @potench XD

ldfloresgon commented 6 years ago

what do you think about this implementation? thanks image

potench commented 6 years ago

I dunno, does it work for you? What kind of feedback are you looking for exactly?

  1. googletag might not exist in componentDidMount yet
  2. all those lets can be consts
  3. Look at this https://github.com/nfl/react-gpt/blob/8278236156b11ba35619bede3e32219d4c24f41b/src/Bling.js#L457-L470 to get some inspiration for managing viewability and seems like GPT takes care of the first ad render?
  4. You should provide a width/height to the <div> so your site doesn't shift as the ad renders.

I'm gonna close this issue since it's something that might be better off posted in Stackoverflow.