Add tweet status column for supported post types #121
Conversation
There was a problem hiding this comment.
Hi @thrijith, thank you for your PR, it looks good to me. However, there are some minor issues in your PR, can you please fix them to get this ready to be merged:
- The post filter hasn't been added. I notice you set this PR as draft, so you may be working on it.
- The tweeted icon in the table header shouldn't have a hover state.
- The tweeted icon doesn't really match the one in the design to me, this one looks closer.
- When you click on the tweeted icon (which opens the tweet page in a new tab), there are two blue dots on the left of the clicked icon.
includes/core.php
Outdated
| $status = isset( $tweet_status['status'] ) ? $tweet_status['status'] : ''; | ||
|
|
||
| if ( 'publish' !== $post_status || empty( $status ) ) { | ||
| echo ''; |
There was a problem hiding this comment.
This echo call can be eliminated by check for the opposite conditions.
There was a problem hiding this comment.
@dinhtungdu updated, please check again.
includes/core.php
Outdated
| // Loop through all the supported post types and add tweet status column. | ||
| $post_types = Utils\get_enabled_post_types(); | ||
| foreach ( (array) $post_types as $post_type ) { | ||
| add_post_type_support( $post_type, POST_TYPE_SUPPORT_FEATURE ); |
There was a problem hiding this comment.
this duplicates the functionalities of set_post_type_supports()
There was a problem hiding this comment.
@dinhtungdu I've moved similar code to the other int hook function we already had.
includes/core.php
Outdated
| '<a href="' . esc_url( $twitter_url ) . '" target="_blank"> | ||
| <span class="autoshare-for-twitter-status-logo" title="' . esc_attr( $tweet_title ) . '"></span> |
There was a problem hiding this comment.
title attribute should be added to <a> instead.
There was a problem hiding this comment.
@dinhtungdu updated, please check again.
includes/core.php
Outdated
| * @param int $post_id Post ID. | ||
| */ | ||
| function modify_post_type_add_tweet_status( $column_name, $post_id ) { | ||
| if ( 'is_tweeted' === $column_name ) { |
There was a problem hiding this comment.
Using early return style can save us up to 2 indent levels.
There was a problem hiding this comment.
@dinhtungdu updated, please check again.
Allow hover change on post data row only
Yes, currently checking for options to do the same ref #56 (comment)
Done.
Unable to download the linked one, we need two for each hover and inactive state, not getting the issue with current icons.
Done. |
Description of the Change
Adds changes outlined in #56
Verification Process
feature/add-custom-filterNormal view
Hover view
Checklist:
Applicable Issues
Fixes #56
Changelog Entry
Add Tweet status column for supported / enabled post types.