Co-Authors-Plus icon indicating copy to clipboard operation
Co-Authors-Plus copied to clipboard

Fixed missing guest-author avatar in select co-author widget

Open nmeh20 opened this issue 7 years ago • 1 comments

The select co-author widget in the admin panel was not showing guest author avatar image. I have made few changes in the code to fix this issue.

nmeh20 avatar Oct 26 '17 01:10 nmeh20

Hi @nilzari Thanks for the patch! I did have a few questions. Can you elaborate on what issue this is fixing? I'm having a hard time seeing what was broken. Could you walk thru the steps to reproduce the original problem and then could you elaborate on what you changed and why?

For example to test I created a new co-authors account with an email I have attached to a new gravatar. When on wp-admin/post-new.php and using the widget at the bottom I searched for that user. The avatar then showed up after selecting the user. After the patch was applied I couldn't notice a difference.

I'm seeing a bit of the formatting not following the WordPress coding standards, for example spaces instead of tabs, the spacing inside functions and the like. There's also a blank line that changed ( https://github.com/Automattic/Co-Authors-Plus/pull/459/files#diff-c3e53bfa31627283686e19232d9e3841L196) which is spacing. If formatting changes are warranted we recommend having them in a separate pull request so that it's easier to see.

You can use the coding standard built into some IDE such as PHPStorm you can use the WordPress standard. You can also use PHPCS https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards On that topic, PHPCS would also help pick up things such as the missing escaping on L1105 of co-authors-plus.php the value of $thumbnail should be escaped on output. We apply late escaping to everything, even when it comes from "safe" functions. You can read more about it here: https://vip.wordpress.com/documentation/validating-sanitizing-escaping/#always-escape-late

One thing I had missed that @rebeccahum pointed out to me is that the .noshow class seems to be added in this commit but isn't being used. Was this part of some testing you were doing? How does it relate to the hidden span that you added?

Thanks for the commit and looking forward to your reply!

sboisvert avatar Nov 11 '17 18:11 sboisvert

Closing for lack of response.

GaryJones avatar Jul 27 '23 21:07 GaryJones