Summary: | Add support for categories and custom post types to the social meta plugin | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jon Davis <jond> | ||||||||
Component: | WebKit Website | Assignee: | Jon Davis <jond> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, hi, jond, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Jon Davis
2020-01-15 17:05:05 PST
Created attachment 387874 [details]
Patch
Comment on attachment 387874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387874&action=review r=me > Websites/webkit.org/wp-content/plugins/social-meta.php:35 > + $description = get_bloginfo('name') . " blog archive."; NIT: should we use "category" instead of `is_category()`? > Websites/webkit.org/wp-content/plugins/social-meta.php:42 > + $url = get_category_link($category_term_id); Should we only override `$url` if `get_category_link(...)` returns a non-empty string (which indicates an error)? > Websites/webkit.org/wp-content/plugins/social-meta.php:49 > + $post_type_obj = get_post_type_object( $post_type ); Style: extra spacing around `(` and `)` > Websites/webkit.org/wp-content/plugins/social-meta.php:50 > + Style: unnecessary newline > Websites/webkit.org/wp-content/plugins/social-meta.php:51 > + if ( isset( $post_type_obj->description ) ) Ditto (49) Also, should we null-check `$post_type_obj` too in case there was an error? > Websites/webkit.org/wp-content/plugins/social-meta.php:54 > + $url = get_post_type_archive_link($post_type); Ditto (42) Created attachment 387935 [details]
Patch
Comment on attachment 387935 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387935&action=review r=me, but this needs to be rebased > Websites/webkit.org/wp-content/plugins/social-meta.php:32 > + $description = "A repository of all ".get_bloginfo('name')."blog posts."; Style: I think we should have spaces around `.` Also, I think there should be a space before "blog". > Websites/webkit.org/wp-content/plugins/social-meta.php:48 > + $title = get_bloginfo('name')." Blog ".get_the_archive_title(); > + $description = "A collection of ".get_bloginfo('name')." blog posts."; Ditto (32) > Websites/webkit.org/wp-content/plugins/social-meta.php:60 > + preg_match_all('/<img.+src(set)?=[\'"]([^\s\'"]+).*[\'"].*>/i', $post->post_content, $matches); This means that we will never use dark mode images. Is that OK, or do we prefer that instead? > Websites/webkit.org/wp-content/plugins/social-meta.php:61 > + if (isset($matches[2][0])) $image_url = $matches[2][0]; Style: please put the body of the `if` on a separate line. Created attachment 387950 [details]
Patch
Comment on attachment 387950 [details]
Patch
r=me, thanks for iterating!
Comment on attachment 387950 [details] Patch Clearing flags on attachment: 387950 Committed r254709: <https://trac.webkit.org/changeset/254709> All reviewed patches have been landed. Closing bug. |