Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

New: Add Ethical Ads #766

Merged
merged 3 commits into from
Aug 17, 2020
Merged

New: Add Ethical Ads #766

merged 3 commits into from
Aug 17, 2020

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Aug 11, 2020

This ads Ethical Ads to each page (also removes the banner). The fallback if there is no ad, or if JS is disabled, is to show an ad for sponsoring ESLint.

I have an open question to them about how the ad looks on the homepage as it seems there's some weirdness with the CSS not being caused by us.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Aug 11, 2020
@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of the website and removed triage An ESLint team member will look at this issue soon labels Aug 11, 2020
@netlify
Copy link

netlify bot commented Aug 11, 2020

Deploy preview for eslint ready!

Built with commit 6d13d54

https://deploy-preview-766--eslint.netlify.app

Copy link
Member

@btmills btmills 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 pages except the homepage look good.

Screen Shot 2020-08-12 at 11 05 38 PM

Is this what you mean by CSS weirdness on the homepage? I tried changing the background to a lighter color just on the homepage, but that looked weird, but making the anchor text lighter (again, just on the homepage) looked okay. Not sure how that would play with ads though.

I'm only seeing the sponsorship fallback on the Netlify deploy, no ads, though the ethicalads script is loading. Just to confirm, is that the expected behavior for the time being?

@kaicataldo
Copy link
Member

I know this is a WIP - so this might be in the pipeline - but the layout on a few other pages looks a little wonky with the floated ad container as well:

Team page:
Screen Shot 2020-08-12 at 11 46 31 PM

Sponsors page:
Screen Shot 2020-08-12 at 11 45 30 PM

@nzakas
Copy link
Member Author

nzakas commented Aug 13, 2020

@btmills yeah, that's the homepage weirdness I was talking about. I figured out how to fix it. For the others, if you are only seeing the sponsorship fallback, it's possible you have an ad blocker on your browser. The sponsorship fallback will only be shown if the ad doesn't load for some reason.

@kaicataldo oops, missed those pages. I'll take a look.

@nzakas
Copy link
Member Author

nzakas commented Aug 13, 2020

Okay, this should be set now. Thanks for the feedback.

@nzakas nzakas marked this pull request as ready for review August 13, 2020 21:27
@kaicataldo
Copy link
Member

LGTM! I don't there's enough contrast with the ad on the hero of the home page, but that's an existing problem with the current header text as well. Maybe we can find an alternate background image (or scrap the image altogether), but that shouldn't block this.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM! I really like how you did the text ad on the demo and sponsors pages.

@nzakas
Copy link
Member Author

nzakas commented Aug 15, 2020

Okay, I’ll plan on merging on Monday so we can finalize the setup. No sense in having three days of placeholder ads on the site over the weekend.

@nzakas nzakas merged commit 957eea4 into master Aug 17, 2020
@nzakas nzakas deleted the ads branch August 17, 2020 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants