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

Donate Block: improve donate block appearance for WP 5.8 #1398

Merged
merged 2 commits into from
Jul 5, 2021

Conversation

laurelfulford
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

This PR cleans up some small issues in the Donate Block when placed in the widget block spaces in WordPress 5.8, specifically making sure it's still picking up the custom colours, and reducing the padding a bit when it's in narrower spaces.

See #1372

How to test the changes in this Pull Request:

  1. Start on a test site running the latest WordPress 5.8 RC; you can do this using the Beta Tester plugin.
  2. Enable the slide-out sidebar, and set those widgets to also appear in the mobile menu.
  3. Set up your site to use custom colours and fonts; set the footer to use a darker custom footer colour - this will help us test contrast, and that the block is picking up the custom colours when appropriate.
  4. Add the Donate widget to every widget space: sidebar, footer, slide-out sidebar, above copyright, and above and below the content area.
  5. View on the front end; note that the block is not always picking up the custom colours, and in some cases there's a bit more padding than needed:

Sidebar - spacing:
image

Slide-out Sidebar - spacing and colour:
image

Slide-out Sidebar widgets in the mobile area - spacing and colour:
image

Footer and Above Copyright - colour; weird 'thank you' spacing:
image

  1. Apply the PR and run npm run build.
  2. Confirm that the above issues are resolved:

Sidebar:

image

Slide-out Sidebar:

image

Slide-out Sidebar widgets in the mobile area:

image

Footer and Above Copyright:

image

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@laurelfulford laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Jul 2, 2021
@laurelfulford laurelfulford requested a review from a team July 2, 2021 18:53
@@ -232,6 +232,27 @@ p.has-background {
}
}

//! Newspack Donate Block
Copy link
Member

Choose a reason for hiding this comment

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

More of a general remark, but I find it a bit weird that there are block styles in the theme. Why are these not in newspack-blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good question! I opted to put these styles in the theme instead of the plugin because they require the theme, if that makes sense. If it was a more general style that was always applied to the Donate block I agree it should go in the plugin. In this case, the block needs to be in certain areas of the theme (the sidebar, the mobile menu) for the styles to be applied, and with any other theme, these styles wouldn't be used. It's kind of similar to the custom colour/typography styles living in the theme instead of the plugin - even though those styles are applied to the block, it's only in the context of this theme, and wouldn't be useful otherwise.

Let me know if this doesn't make sense as an approach!

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that makes sense, thanks for the clarification!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Jul 5, 2021
@laurelfulford
Copy link
Contributor Author

Thanks Adam! I'm going to go ahead and merge this but I'm open to discussing the location of these styles further if my logic doesn't make sense :)

@laurelfulford laurelfulford merged commit 51fc4f7 into master Jul 5, 2021
@laurelfulford laurelfulford deleted the fix/1372-donate-block-wp-5-8 branch July 5, 2021 16:15
matticbot pushed a commit that referenced this pull request Jul 6, 2021
## [1.39.3-alpha.1](v1.39.2...v1.39.3-alpha.1) (2021-07-06)

### Bug Fixes

* ads media queries ([239ff6e](239ff6e))
* correct custom fonts in block widget preview ([#1401](#1401)) ([878b6a1](878b6a1))
* figcaption rendering bug in Safari ([#1382](#1382)) ([e274499](e274499))
* improve donate block appearance for WP 5.8 ([#1398](#1398)) ([51fc4f7](51fc4f7))
* prevent white screen and errors on Widget Screen ([#1390](#1390)) ([5731581](5731581))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.39.3-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jul 6, 2021
## [1.39.3](v1.39.2...v1.39.3) (2021-07-06)

### Bug Fixes

* ads media queries ([239ff6e](239ff6e))
* correct custom fonts in block widget preview ([#1401](#1401)) ([878b6a1](878b6a1))
* figcaption rendering bug in Safari ([#1382](#1382)) ([e274499](e274499))
* improve donate block appearance for WP 5.8 ([#1398](#1398)) ([51fc4f7](51fc4f7))
* prevent white screen and errors on Widget Screen ([#1390](#1390)) ([5731581](5731581))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.39.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants