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

Do not have a separate background for ads #3625

Closed

Conversation

davidfischer
Copy link
Contributor

If we do want to have a separate background for ads I would say we should be systematic about it. Also, this makes it harder to build an ad for more themes.

BEFORE
screen shot 2018-02-16 at 11 18 20 am

AFTER
screen shot 2018-02-16 at 11 18 13 am

- If we do want to have a separate background for ads
  I would say we should be systematic about it.
- Also, this makes it harder to build an ad for more themes
@ericholscher
Copy link
Member

Seems reasonable. I do think this was done to make the ad feel separate from the content. Is the reason we don't want it because it's harder to port, or some other reason?

@davidfischer
Copy link
Contributor Author

I'm trying to reduce theme specific stuff and especially stuff that is somewhat inconsistent.

@RichardLitt RichardLitt added the Design Design or UX/UI related label Feb 21, 2018
@agjohnson
Copy link
Contributor

I'm impartial on removing these styles, but I do prefer the background color for differentiation on our theme and on alabaster. The changes here look fine if this is a change we need to make though.

@davidfischer
Copy link
Contributor Author

This changed anyway with the rework for the Ads API #3747. This can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Design or UX/UI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants