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

feat: allow different post types in carousel; plus new block options #746

Merged
merged 5 commits into from
Apr 30, 2021

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Apr 29, 2021

All Submissions:

Changes proposed in this Pull Request:

Makes a few changes:

  • Allows any post types with newspack_blocks support to be selected and added to a Posts Carousel, much like with the Homepage Posts block.
  • Enables Specific Posts mode for the carousel, just like with Homepage Posts.
  • On the front-end, if a post has an external sponsor URL meta value defined, the slide will link to this, otherwise it will link to the post's permalink. This is to accommodate sponsors, who don't have working permalinks but should have an external sponsor URL set. If it turns out that some sponsors don't have that metavalue set, we can add a block attribute to choose whether to link the slides at all.
  • Adds some new block options:
    • imageFit – Lets you specify whether the image is cropped to fill the slider or downsized to fit within it. The latter is useful for showing sponsor logos that you don't want to be visually cropped.
    • showTitle – Lets you show or hide the title. Now, if you toggle off all options under "Article Meta Settings", the meta container at the bottom of each slide with the translucent black background is not rendered at all, which should be useful if you want to show sponsor logos with no other information.

You must have this change active in the Newspack Sponsors plugin if you want to test with the sponsor CPT: Automattic/newspack-sponsors#52

Closes #745.

How to test the changes in this Pull Request:

  1. Check out this branch, run npm run build to build files.
  2. On a post or page, add a Post Carousel block.

Post Types options

  1. In the editor, select the Post Carousel block and confirm that there's a new "Post Types" sidebar panel. Expand it.
  2. Confirm that there are several post type options that can be toggled on here, with "Posts" being the only one selected by default. Note that the options you see here depend on which other Newspack plugins you have active on your site:
  • If you have Newspack Newsletters active, you'll see an option "Newsletters"
  • If you have Listings active, you'll see options for "Generic Listings", "Events", "Marketplace Listings", and "Places"
  • If you have Sponsors active, you'll see an option for "Sponsors" (must have feat: add support for Newspack Blocks newspack-sponsors#52 active for that plugin)
  • At the very least, you'll see options for "Posts" and "Pages".
  1. Toggle on and off each option and confirm that when the option is on, its corresponding post types a.) are shown in the preview and on the front-end query results, and b.) can be searched for if the block is in "Specific Posts" mode (see below for testing on that).
  2. Note that other post types can be added here as options by adding support for newspack_blocks to the post type, e.g. add_post_type_support( 'cpt_name', 'newspack_blocks' );

Specific Posts mode

  1. In the editor, select the Post Carousel block and expand the "Display Settings" sidebar panel. Confirm that there's now a "Choose Specific Posts" toggle here:

Screen Shot 2021-04-29 at 4 58 38 PM

  1. Toggle it on and enter post titles to search for them. Select one or more posts and confirm that it works as expected, and that the carousel in the preview and on the front-end shows only the posts selected if in specific posts mode.

Image Fit option

  1. In the editor, select the Post Carousel block and expand the "Slideshow Settings" sidebar panel. Confirm that there's now an "Image Fit Style" setting with two options, with "Cover" being the default (which means that existing carousels won't change their behavior):

Screen Shot 2021-04-29 at 5 02 09 PM

  1. Set the option to "Contain" and confirm that the carousel now renders its images centered and uncropped, instead of cropping them to fill the space. Confirm in both the editor and on the front-end.

Show Title option

  1. In the editor, select the Post Carousel block and expand the "Article Meta Settings" sidebar panel. Confirm that there's a new "Show Title" setting which is enabled by default.

Screen Shot 2021-04-29 at 5 04 19 PM

  1. Toggle the setting off and confirm that the carousel renders without showing post titles in both the editor and front-end.
  2. Toggle off all settings under "Article Meta Settings" and confirm that the carousel now renders without any entry-meta info, or its translucent black background, in both editor and front-end:

Screen Shot 2021-04-29 at 5 05 41 PM

Sponsor links

Sponsor CPTs don't have working permalinks and are not viewable as public posts by themselves. They do, however, have a "Sponsor URL" meta field, so if this exists sponsor CPTs will link to that instead of the permalink in Post Carousel and Homepage Posts blocks.

Note: It's possible to create a sponsor without defining an external URL? In this situation, the blocks will fall back to the permalink, which will be broken. :\ If this turns out to be a problem I would say we should add a block attribute to not link any slides in the slideshow.

  1. Create and publish several sponsor posts using Newspack Sponsors. Make sure to add Sponsor URLs to them.
  2. Create a Post Carousel and Homepage Posts block.
  3. In each, under Post Types, enable the Sponsors post type. Publish/update the post.
  4. On the front-end, confirm that the sponsor slides link to the Sponsor URLs, not the permalinks.

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?

Copy link
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple small things that need fixing (mostly the title attribute). Great idea with this feature, and thanks for the speedy implementation!

src/blocks/carousel/view.php Outdated Show resolved Hide resolved
src/blocks/carousel/view.php Show resolved Hide resolved
@dkoo dkoo requested a review from claudiulodro April 30, 2021 15:54
Copy link
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

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

Latest version works great!

@dkoo dkoo merged commit f30a5a5 into master Apr 30, 2021
@dkoo dkoo deleted the feat/post-types-in-carousel branch April 30, 2021 21:51
matticbot pushed a commit that referenced this pull request May 4, 2021
# [1.25.0-alpha.1](v1.24.0...v1.25.0-alpha.1) (2021-05-04)

### Features

* allow different post types in carousel; plus new block options ([#746](#746)) ([f30a5a5](f30a5a5))
@matticbot
Copy link
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request May 5, 2021
# [1.25.0](v1.24.0...v1.25.0) (2021-05-05)

### Features

* allow different post types in carousel; plus new block options ([#746](#746)) ([f30a5a5](f30a5a5))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.25.0 🎉

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Posts Carousel: Allow additional post types
3 participants