WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 years ago

#16173 accepted enhancement

Make the Calendar widget support custom post types

Reported by: GautamGupta Owned by: GautamGupta
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Widgets Keywords: gci needs-testing needs-patch
Focuses: Cc:

Description

The Calendar widget should have support for custom post types (much like how the tag cloud widget supports custom taxonomies) so that the plugins could just have supports => array( ... 'calendar' ... ) and the calendar would display that post type in the option box (this is just an example, used in the patch I'm gonna attach, we may use some other arg, of course).

Attachments (3)

calendar.diff (7.3 KB) - added by GautamGupta 4 years ago.
TODO: Something in the place of archive links
calendar.2.diff (18.7 KB) - added by GautamGupta 3 years ago.
16173.patch (10.9 KB) - added by sushkov 3 years ago.
3rd take

Download all attachments as: .zip

Change History (13)

GautamGupta4 years ago

TODO: Something in the place of archive links

comment:1 GautamGupta4 years ago

  • Cc gautam.2011@… added
  • Keywords gci added
  • Status changed from new to accepted

comment:2 johnjamesjacoby4 years ago

  • Cc johnjamesjacoby@… added

This comes from the idea of having a topic calendar in the bbPress plugin for WordPress. A Google Code-in task was added, and this patch is the result of that task.

Last edited 4 years ago by johnjamesjacoby (previous) (diff)

comment:3 mdawaffe3 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release

I like the idea.

The wpdb calls in the patch should use wpdb::prepare(). I think "_get_instance_post_type" would be a better name than "_get_current_post_type".

comment:4 GautamGupta3 years ago

Attached an improved patch based on the suggestions. Also contains some code cleanup.

GautamGupta3 years ago

comment:5 Doggie523 years ago

  • Cc douglas@… added

I presume displaying this patched calendar is simply by doing get_calendar( , , "cpt") ? Very much a good idea - hope this gets some attention.

comment:6 sushkov3 years ago

  • Cc sushkov added
  • Keywords has-patch dev-feedback added; bbpress needs-patch removed
  • Version changed from 3.1 to 3.3

Attached a new version for this ticket.

What changed:

  • added wpdb::prepare()
  • added proper support for filters and CPT namespaces
  • removed 3rd argument to ensure compatibility with older themes
  • added the class name of the current CPT to make sure designers can differentiate calendars

To note, what would be really cool, is to be able to send an array of post types we want to fetch, though with wpdb::prepare() I couldn't find a way to escape an array (*cough-cough* another ticket?).

The patch for widget can be added by request, though I would opt it be done in a different way but proposed for now: fetching and listing the public post types in a dropdown will reduce modifications across codebase.

sushkov3 years ago

3rd take

comment:7 johnjamesjacoby3 years ago

  • Keywords needs-testing added

Still all for this. There is interest and the patch is straight forward.

Adding needs-testing keyword to get more eyes on it.

comment:8 SergeyBiryukov3 years ago

  • Milestone changed from Future Release to 3.3
  • Version changed from 3.3 to 3.1

comment:9 westi3 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

Notes from reviewing all the patches and ideas here:

  • I'm not sure the core Calendar widget should gain CPT support at this point. I think bbPress is probably more of a special case in CPT usage in that it is doing something chronological - normal usage of CPT's would be for other types of content that are not chronological.
  • I like the improvements to get_calendar a lot and I don't have a problem with adding CPT support to that as an API function.

Detailed review notes and feedback for get_calendar improvements in 16173.patch - these assume we keep support for CPTs in the function.

  • the new cache key naming means that the cache is no longer cleared for the post post_type and will require extra logistics for CPTs - we should clear the cache for all CPTs on the correct hooks I guess
  • With the patch applied I get no previous month link on my test site (has no posts this month but some last month)
  • We shouldn't rename the get_calendar filter - but rather pass extra arguments to it so that a plugin can be CPT aware or use the old name for posts and a new CPT specific name for other CPTs.
  • We should probably switch to using wp_count_posts() for the have we got any posts check rather than a direct SQL query.
  • We should check if $ak_title_separator logic is still needed and if so convert the checks to use the $is_{browser} variables

Going forward it would be good to see:

  • An improved patch which resolves the above issues and doesn't support CPTs
  • An improved patch which resolves the above issues and does support CPTs in get_calendar in a backward compatible manner - i.e. preserving filter and cache key names.

I don't think we should do the widget at this time.


comment:10 jane3 years ago

  • Milestone changed from 3.3 to Future Release

The standard message around this point in the cycle: "If this is making it into 3.3, it needs to be committed now, as we have passed freeze. If it's not ready to be committed, punt to next cycle."

Since the last comment on this was from a lead dev saying not right now, and no other lead devs stepped forward to say it should go in, am punting for now. Can keep working on the patch per westi's suggestions to target early 3.4.

Note: See TracTickets for help on using tickets.