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.

Sign up for GitHub

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

Jump to bottom

Remove white background from Jetpack Ads block #13716

Merged
merged 1 commit into from
Oct 11, 2019
Merged

Remove white background from Jetpack Ads block #13716

merged 1 commit into from
Oct 11, 2019

Conversation

joanrho
Copy link
Contributor

@joanrho joanrho commented Oct 10, 2019

Changes proposed in this Pull Request:

This removes the white background from the Jetpack Ads block.

Fixes #13715

Before:
Screen Shot 2019-10-10 at 2 43 12 PM
After:
Screen Shot 2019-10-10 at 2 43 36 PM

Testing instructions:

  • Check out this PR
  • Add a Jetpack Ads block to any post
  • See that there is no white background color in the block like in the above "After" mockup.

Proposed changelog entry for your changes:

  • None needed.

@joanrho joanrho added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Design Review Complete [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Oct 10, 2019
@joanrho joanrho requested review from MichaelArestad and a team October 10, 2019 21:03
@joanrho joanrho self-assigned this Oct 10, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello joanrho! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33894-code before merging this PR. Thank you!

@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against 8f88405

@joanrho joanrho added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Oct 10, 2019
MichaelArestad
MichaelArestad approved these changes Oct 10, 2019
View reviewed changes
Copy link
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

Works like a charm. I wonder why that was there in the first place.

@jeherve jeherve added this to the 7.9 milestone Oct 11, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 11, 2019
@jeherve jeherve merged commit 0300564 into master Oct 11, 2019
@jeherve jeherve deleted the update/jetpack-ad-block-background branch October 11, 2019 11:55
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 11, 2019
@simison
Copy link
Member

simison commented Oct 11, 2019

How does it look like in Gutenberg dark mode?

You can test on dark mode using starter theme https://github.com/WordPress/gutenberg-starter-theme (dark mode can be activated from "theme settings")

@jeherve
Copy link
Member

jeherve commented Oct 11, 2019

It looks like this:
image

Here is how it looked before the changes:
image

@simison
Copy link
Member

simison commented Oct 11, 2019 •
edited
Loading

Nice! :-) Thanks for checking!

The icon SVG should prolly have fill="currentColor" to show up as white as well.

jeherve added a commit that referenced this pull request Oct 11, 2019
@jeherve
Ad Block: add fill color to Ad block icon.
7e5a4d9 
See #13716 (comment)
@jeherve jeherve mentioned this pull request Oct 11, 2019
Merged
@jeherve
Copy link
Member

jeherve commented Oct 11, 2019

@simison Agreed. This should do it: #13727

jeherve added a commit that referenced this pull request Oct 23, 2019
@jeherve
Changelog: add #13716
8fc1675
jeherve added a commit that referenced this pull request Oct 28, 2019
@jeherve
Ad Block: add fill color to Ad block icon. (#13727)
5e761ba 
See #13716 (comment)
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelArestad MichaelArestad MichaelArestad approved these changes

@jeherve jeherve jeherve approved these changes

Assignees

@joanrho joanrho

Labels
[Block] Ad [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Design Review Complete Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
7.9
Development

Successfully merging this pull request may close these issues.

Jetpack Ads block - remove white background color
6 participants
@joanrho @matticbot @jetpackbot @simison @jeherve @MichaelArestad