Opened 10 years ago

Closed 9 years ago

#610 closed enhancement (fixed)

Add post types to 'At a Glance' dashboard widget

Reported by: hlashbrooke's profile hlashbrooke Owned by: iandunn's profile iandunn
Milestone: Priority: low
Component: WordCamp Site & Plugins Keywords: has-patch commit


Pretty self-explanatory really - it would be useful to have the WC post types (speakers, sessions and sponsors) as well as the CampTix attendees on display in the 'At a Glance' widget in the dashboard. The attached patch adds them in and gives them the correct icons.

Attachments (3)

610.diff (3.3 KB) - added by hlashbrooke 10 years ago.
Patch as described in ticket body.
610.1.diff (3.3 KB) - added by hlashbrooke 10 years ago.
Updates as per previous comment
610-meta.diff (3.0 KB) - added by kovshenin 9 years ago.

Download all attachments as: .zip

Change History (8)

10 years ago

Patch as described in ticket body.

#1 follow-up: @iandunn
10 years ago

  • Owner set to iandunn
  • Priority changed from normal to low
  • Status changed from new to accepted

This sounds like a good idea. A few comments on 610.diff:

  • tix_attendee isn't part wc-post-types, so I'm leery of including that in glance_items(); it seems wrong to couple the two plugins together. It's not ideal to duplicate the code in both plugins either, but since CampTix is distributed as a generic plugin -- as opposed to only being used on -- then I think that's probably the best compromise in this situation.
  • The text domain we use is wordcamporg.
  • The coding standards now call for braces around all conditions (line 1665), and spaces between control structures and their opening parentheses.
  • On 1665, do we want if ( isset( $num_posts->publish ) && $num_posts->publish ) instead? if ( $num_posts ) will always evaluate to true, because wp_count_posts() always returns an object.

#2 in reply to: ↑ 1 @hlashbrooke
10 years ago

Thanks for the feedback Ian.

  • I agree about the 'tix_attendee' post type - would be better to add that to CampTix itself. I'll submit a pull request on GitHub (only just saw now that the repo is public).
  • Sorry about the textdomain typo - I copied the function from one I wrote a while ago that I like to keep generic, but just forgot to replace that string.
  • Braces added for the continue; command - leaving them out is a habit that I've been trying to break out of lately :)
  • I think you're right about the $num_posts conditional - I'll change that to be more verbose.

Will add the updated patch after this.

10 years ago

Updates as per previous comment

9 years ago

#3 @kovshenin
9 years ago

  • Keywords commit added

Hi there! Thanks for your patch @hlashbrooke, I refreshed it against the current version in 610-meta.diff, also cleaned up some space vs tabs, changed the $type vs $post_type vs $post_type_object to follow what core uses.

And also a note about the usage of _n() and other localization functions - you can't really construct strings from variables, all strings passed to such functions should actually be strings for translation tools to pick them up. That said, we could have used the singular and name labels for the registered post types, however, for some languages plural has more than one form, which means we can't. I'm pretty sure there's a core ticket to address this, but I can't find it right now. A workaround is to use hard-coded strings, which is what core does for posts and pages.

This patch is ready for commit, but an extra couple eyes on it won't hurt.

#4 @hlashbrooke
9 years ago

Thanks @kovshenin!

I actually did the localisation that way because I assumed the name labels for the post types would already be localised, but you're right - the new patch makes things better for more languages.

#5 @kovshenin
9 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 1026: Add speakers, sessions and sponsors to the At a Glance Dashboard widget.

Fixes #610.
Props hlashbrooke.

Note: See TracTickets for help on using tickets.