Show theme, plugin or author in Added By column with appropriate icon or avatar #36763
Conversation
talldan
added
[Type] Enhancement
| .edit-site-list-added-by__icon { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| width: $grid-unit-40; | ||
| height: $grid-unit-40; | ||
| background: $gray-800; | ||
| border-radius: 100%; | ||
|
|
||
| svg { | ||
| fill: $white; | ||
| } | ||
| } | ||
|
|
||
| .edit-site-list-added-by__avatar { | ||
| width: $grid-unit-40; | ||
| height: $grid-unit-40; | ||
| border-radius: 100%; | ||
| } |
There was a problem hiding this comment.
@jameskoster Would be great to get an early check on these styles. Feel free to push a commit or comment with any adjustments you want to make.
There was a problem hiding this comment.
Styles looking good so far. I did notice that customised theme templates are displaying strangely, but assume you're still working on that?
There was a problem hiding this comment.
Yeah, I noticed it's a bit glitchy when loading the image too. I'll look at improving it 👍
There was a problem hiding this comment.
This should be fixed now.
|
Size Change: +994 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
| // Fallback for any custom template already created without an assigned | ||
| // author. | ||
| if ( | ||
| ! template.has_theme_file && | ||
| template.source === 'custom' && | ||
| ! template.author | ||
| ) { | ||
| return ( | ||
| <HStack alignment="left"> | ||
| <div className="edit-site-list-added-by__icon"> | ||
| <Icon icon={ customizedByUserIcon } /> | ||
| </div> | ||
| <span>Customized by user</span> | ||
| </HStack> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Currently this is what displays for any templates that were customized but don't have an author:
I think even if we add author support to templates and template parts, there will still be some that have been created by Gutenberg plugin users up until now that won't have an author, so we need to handle this case.
@jameskoster Maybe the best option in this situation would be to show the theme icon and name, but still with the blue customized dot?
There was a problem hiding this comment.
Currently this is what displays for any templates that were customized but don't have an author:
The important thing to communicate here is that the template was created from scratch. In lieu of a specific author, perhaps we could just say it was added by the site, and use the site logo as the graphic?
There was a problem hiding this comment.
Managed to make that work 👍
|
I have a core patch for adding author support to template and template parts - WordPress/wordpress-develop#1937. With that checked out and this checked out it should be possible to create templates or template parts, when viewing them on the listing screen the author should show up in the Added By column. |
talldan
force-pushed
the
update/added-by-in-templates-list
branch
from
a815bbc
to
6d9c718
Compare
talldan
added
the
Backport to WP Beta/RC
talldan
requested review from
nerrad,
spacedmonkey and
TimothyBJacobs
as code owners
|
This should be in a fairly good state now for more testing and feedback. |
|
Hmm, I'm seeing some unexpected behavior.
|
talldan
force-pushed
the
update/added-by-in-templates-list
branch
from
9da9eea
to
dc91102
Compare
|
@jameskoster That's unusual, I haven't changed anything related to how that delete action works. I can't reproduce the issue you're having, my page still looks as expected. edit: It may have been related to a problem in |
talldan
force-pushed
the
update/added-by-in-templates-list
branch
from
dc91102
to
397b0a6
Compare
talldan
force-pushed
the
update/added-by-in-templates-list
branch
from
397b0a6
to
e8a881a
Compare
|
The REST API changes are at risk of not landing in time for 5.9, so I've separated out the REST API changes from this PR (#36896 now contains them). It then becomes a pure UI change. I've already made it so that it gracefully handles the existing template data as much as possible:
|
| @@ -13,6 +13,12 @@ function gutenberg_register_template_part_post_type() { | |||
| return; | |||
| } | |||
|
|
|||
| // If the post type has already been registered (by WordPress core), skip | |||
| // registration. | |||
| if ( post_type_exists( 'wp_template_part' ) ) { | |||
There was a problem hiding this comment.
I split this and the similar change in templates.php out into a separate PR (#36898), but it's still needed here to be able to test alongside the WP core changes.
There was a problem hiding this comment.
This change breaks the site editor for WP 5.8. I also see unexpected behavior that James mentioned (#36763 (comment)) if I access the list page directly.
I think the reason is the difference between REST controllers, and since we have to support 5.8, it might be better to remove this check. What do you think?
Screenshot
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
I also see unexpected behavior that James mentioned (#36763 (comment)) if I access the list page directly.
Do you see that only in 5.8?
I think we should consider updating the Gutenberg version of the REST API. Maybe we can also add a version check as part of this if statement.
There was a problem hiding this comment.
If I remove post type exists checks everything works correctly. The endpoint in WP 5.8 only supports user-created templates.
There was a problem hiding this comment.
Ok, I've reverted this for now, will move the conversation over to #36898.
It does make it difficult to test this with the associated REST changes, best thing is to revert it back again if you want to do that.
There was a problem hiding this comment.
I also see unexpected behavior that James mentioned (#36763 (comment)) if I access the list page directly.
I also haven't been able to repro this at all, not even in 5.8. Any additional details would be great.
There was a problem hiding this comment.
Were you able to reproduce Site Editor failure on 5.8? I will try to provide additional details.
There was a problem hiding this comment.
Yeah, I did reproduce that. Hopefully it's not a problem now that I've reverted the commit. Thanks for testing it 👍
Still not sure how to do this, but based on the way plugin templates are described to work, it shouldn't be an issue.
I'll move preloading into a separate PR, it isn't straightforward and I don't want to make this more complicated than it already is. |
* Add success and error snackbars to the list page * Fix Unit Test error * Apply 'is-navigation-open' to interface skeleton * Fix NavigationToggle unit test * Apply suggestions from code review Co-authored-by: Robert Anderson <robert@noisysocks.com> * Use deleting * Remove snackbars for creation * Add notices for reverting * Fix test * Add getLastEntityDeleteError * Add comment Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Robert Anderson <robert@noisysocks.com>
… REST endpoints" This reverts commit 6b40260.
talldan
force-pushed
the
update/added-by-in-templates-list
branch
from
e7c055f
to
fdde4c8
Compare
| return () => { | ||
| node.removeEventListener( 'load', onLoad ); | ||
| }; | ||
| }, [] ); |
There was a problem hiding this comment.
onLoad should be in the dependencies. I'm not sure why we need this? Couldn't we just use onLoad attribute on <img>?
There was a problem hiding this comment.
Good point! I'll update it.
noisysocks
removed
the
Backport to WP Beta/RC
… or avatar (#36763) * Show theme, plugin or author in Added By column with appropriate icon or avatar * Add success and error snackbars to the templates list page (#36808) * Add success and error snackbars to the list page * Fix Unit Test error * Apply 'is-navigation-open' to interface skeleton * Fix NavigationToggle unit test * Apply suggestions from code review Co-authored-by: Robert Anderson <robert@noisysocks.com> * Use deleting * Remove snackbars for creation * Add notices for reverting * Fix test * Add getLastEntityDeleteError * Add comment Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Robert Anderson <robert@noisysocks.com> * Show theme, plugin or author in Added By column with appropriate icon or avatar * Fix plugin name rendering * Add blue dot for customize templates * Add tooltip and tidy up logic * Fallback to showin slug * Handle missing author by showing template as a customized theme template * Fallback to site name and logo * Make the avatars load in a fancy way * Avoid icon stretch on narrow viewports * Fix dodgy rebase * Use core version of template and template part post types and REST endpoints * Revert "Use core version of template and template part post types and REST endpoints" This reverts commit 6b40260. * Rename to AddedBy * Use onLoad Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Robert Anderson <robert@noisysocks.com>
|
@paaljoachim I didn't reply to this at the time, but I appreciate the testing.
Previously, templates and template parts didn't support any author data being saved. For that to happen and for it to work a separate change had to land in WordPress core. Your tests would have required a version of WordPress core with that change patched in for the author to display. One of the things you mentioned (the first point) is a bug, so I've made an issue for that - #37086 |
|
Thanks Dan! The current Gutenberg plugin now shows the Added by and user Gravatar. |
Description
This is the start of improving the 'Added by' column in the Templates / Template Parts listing, as described in this comment - #36597 (comment)
There are associated REST API changes that improve how this works - WordPress/wordpress-develop#1937. This comment explains the differences with and without those changes - #36763 (comment).
How has this been tested?
Because template parts and templates currently don't support 'author' the easiest way to test this will be by viewing a list of posts instead of templates:
Customization
3. Customize the template part and return to the page to view it. It should show a blue dot and a tooltip indicating it's customized.Template parts created from scratch
5. If you have the core REST API changes checked out and built (https://github.com/WordPress/wordpress-develop/pull/1937), and repeat these steps, the author should be shown like it does for posts:Types of changes
Enhancement
Checklist:
*.native.jsfiles for terms that need renaming or removal).