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

Allow Custom Post Types to be enabled via admin/customizer #798

Closed
westonruter opened this issue Nov 6, 2017 · 9 comments

Comments

Projects
5 participants
@westonruter
Copy link
Member

commented Nov 6, 2017

[Edited] Refer to the latest acceptance criteria.


As noted in the readme:

By default, the plugin only creates AMP content for posts. You can add support for other post_types using the post_type parameter used when registering the custom post type (assume our post_type is xyz-review):

This requires someone to write code for adding support. However, this ability to opt-in to post type support could instead be presented in the admin (or in the Customizer) so that the user doesn't have to write any code at all.

The list of post types could be presented with checkboxes for deciding whether or not AMP support should be enabled. The checkbox for generic posts would be shown with a checked checkbox that is also disabled since support is built-in and cannot be disabled. After that a checkbox for pages could be presented, once we have pages listed and enabled by default (once we have #176) but then perhaps allow the user to disable support by unchecking it if they don't want it. Then each of the other public post types could be listed, with the checkboxes by default unchecked. Any post types that have hard-coded post type support in the theme or plugin would be disabled and checked.

@jwold

This comment has been minimized.

Copy link

commented Nov 6, 2017

Attached is an annotation showing how this could look when added into the admin settings with the AMP plugin activated:

settings

@westonruter westonruter added this to the v0.6 milestone Nov 7, 2017

@westonruter westonruter added this to To Do in v0.6 Nov 7, 2017

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2017

As with #803, let's merge the analytics into a general AMP settings screen, and we can have it appear under the Settings parent admin menu. We'll start by adding it to the admin screen and then we'll in a later issue move it to the Customizer, along with analytics.

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2017

As with #803, let's merge the analytics into a general AMP settings screen, and we can have it appear under the Settings parent admin menu.

Since the goal is to eventually move to Customizer anyway, let's not worry about #803 here. For the moment we can add a new “AMP > Settings” admin submenu page under the existing AMP menu item, and it can then be the default subitem appearing before Analytics. (Having a separate admin screen for Analytics still makes sense since @ThierryA points out that the fields are repeatable.) This AMP > Settings screen can then have the post type configurations and other settings we add prior to Customizer integration. When Customizer integration does happen, this AMP menu item can just redirect to the AMP panel in the Customizer.

@ThierryA ThierryA moved this from To Do to In Progress in v0.6 Nov 21, 2017

@ThierryA ThierryA self-assigned this Nov 21, 2017

@ThierryA

This comment has been minimized.

Copy link
Collaborator

commented Nov 23, 2017

Acceptance criteria:

  1. A new admin screen will be created to allow for AMP settings management.
  2. The new page will display in the admin left panel listing as a submenu, “General”, within the “AMP” menu.
  3. When selecting “SettingsGeneral” the new AMP admin settings page will be exposed.
  4. The “AMP General” admin page will contain the following:
    • “AMP Settings” page title
    • Post Types Support settings
  5. The “Post Types Support” section will allow a user to select which post types AMP should apply to.
  6. The “Post Types Support” section within the AMP Admin page will include a label “Post Types Support”, and a brief description of how the section is used.
  7. Within the “Post Types Support” section a user will see a listing of the following checkboxes:
    • Posts - will be checked by default and disabled from being unchecked
    • A checkbox foreach public Custom Post Types
    • All post types with the amp post type support declared or removed in a theme or plugin will display an error "on save" should the user try to change the checkbox status.
  8. Deselecting any capable checkbox will disable AMP support for that post type.

@ThierryA ThierryA moved this from In Progress to Ready for Review in v0.6 Nov 23, 2017

@ThierryA ThierryA moved this from Ready for Review to In Progress in v0.6 Dec 6, 2017

@ThierryA ThierryA moved this from In Progress to Ready for Review in v0.6 Dec 6, 2017

@westonruter westonruter moved this from Ready for Review to QA in v0.6 Dec 6, 2017

@jwold

This comment has been minimized.

Copy link

commented Dec 7, 2017

@ThierryA got a question about how this works. Using the following screenshot: http://d.pr/i/xrTNY6.

A. If I checkmark “Bar” will I be forcing AMP to be enabled for this CPT?
B. If I checkmark “Baz” will I be forcing AMP to disable this CPT?

I'm a bit confused about how checking something off could cause different results.

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2017

@jwold sorry, I actually made those parenthetical comments part of the registered post type labels themselves:

image

This was just to help with testing to know that one of the post type is configured to be forced-on, and another with forced-off. Normally these parenthetical comments would not be presented.

@jwold

This comment has been minimized.

Copy link

commented Dec 7, 2017

Ahh, thanks so much! I should have read the CPT in the sidebar to realize.

So if a post type (such as bar) is configured forced-on, what happens if I do checkmark it and what happens if I don't? Same thing with the force-disabled option.

@westonruter

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2017

An error message will be displayed after saving, indicating that your choice was not persisted, and why.

@csossi

This comment has been minimized.

Copy link
Collaborator

commented Dec 8, 2017

AC verified, along with WP error messages re: invalid checking/unchecking

@csossi csossi moved this from QA to Ready for Merging in v0.6 Dec 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.