Chrome: Adding a post author dropdown #2100
Conversation
youknowriad
added
the
General Interface
Codecov Report
@@ Coverage Diff @@
## master #2100 +/- ##
==========================================
- Coverage 20.25% 20.14% -0.11%
==========================================
Files 135 136 +1
Lines 4227 4503 +276
Branches 718 801 +83
==========================================
+ Hits 856 907 +51
- Misses 2842 2987 +145
- Partials 529 609 +80
Continue to review full report at Codecov.
|
|
Looks good! Can you add a
If yes, then ship! |
|
My only concern is the placement. It adds further vertical space onto the already quite lengthy publish box, and its likely a setting which is hardly ever changed. Funnily enough I actually submitted a patch for core 2 years ago that removes the author meta box and puts the option into the publish meta box. However in hindsight I agree with the reason for it not being considered. |
youknowriad
force-pushed
the
add/author-dropdown
branch
from
58d5b9f to
619de3b
Compare
|
Styling fixed. @paulwilde valid concern, there are two designs proposed in the issue and it seemed that the current one has the majority votes. Happy to change it to whichever other option we choose. But maybe we should get this is as a start. |
|
It's a good thought, but worth noting that the dialog itself only shows up if there's more than one author on the site. |
editor/sidebar/post-author/index.js
Outdated
| } | ||
|
|
||
| componentWillUnmount() { | ||
| if ( this.fetchAuthorsRequest ) { |
There was a problem hiding this comment.
I notice in some cases (latest posts, for example) we're checking the status as pending before deciding to abort. If the object let's us call abort anyways, and this property is always defined, maybe we don't need the condition at all?
There was a problem hiding this comment.
right! we probably don't need this condition.
editor/sidebar/post-author/index.js
Outdated
|
|
||
| componentWillUnmount() { | ||
| if ( this.fetchAuthorsRequest ) { | ||
| return this.fetchAuthorsRequest.abort(); |
editor/sidebar/post-author/index.js
Outdated
|
|
||
| fetchAuthors() { | ||
| this.fetchAuthorsRequest = new wp.api.collections.Users().fetch( { data: { | ||
| roles: 'author,editor,administrator', |
There was a problem hiding this comment.
Do we need to define roles here? Two issues
- This doesn't include
contributors(easily spotted for me because I have a site with two users, one of which a contributor 😛 ) - This won't respect custom roles: https://developer.wordpress.org/plugins/users/roles-and-capabilities/#adding-roles
There was a problem hiding this comment.
Good question and I answer with another question: Is is ok to set a "subscriber" as a post author?
There was a problem hiding this comment.
Good question and I answer with another question: Is is ok to set a "subscriber" as a post author?
Sorry, I had been thinking about this the other day and meant to respond: We should request users by capability, not role. Specifically the edit_posts capability. This is not currently supported by the list endpoint, however. Probably an API task worth exploring. I'll create an issue.
youknowriad
dismissed
aduth’s stale review
Dropped the role filter, it looks like we can assign a post to a subscriber
|
I'd appreciate another look here before merging. |
|
I think this looks great! |
closes #968
A simple PR adding a select allowing us to select the post author dropdown in case the "user" request for authors, editors and administrators returns more than one result.