[Document Settings]: Add summary panel version 1 #39973
Conversation
ntsekouras
added
[Type] Enhancement
|
Size Change: +2.3 kB (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
|
Alright, based on work by @javierarce I've pushed some changes, and I think we're fast approaching something. Here's what it looks like now:
Not bad for just a little work! Thoughts:
I'll dive in, but even just as is, this is feeling sweet to me. What do you think, Javi? And thank you Nik for the boost here, looking forward to your thoughts. |
|
It looks like we can remove the I haven't pushed that code because I'm not quite sure what it was there for. @ntsekouras any thoughts? |
|
Just some early notes..
This is might not be needed depending on the changes we are going to make and if we want to apply these to the existing ones. For
Noting that the Author component changes from
I'll have to check the code better for this one. |
|
@jasmussen I just took it for a spin it's working great! Very promising! |
Sounds good, yeah I think the changes made to featured image for this branch are worth it even for the existing component. We could potentially change the excerpt version to "minimal-excerpt" or something like that. But I still think we should remove the panel from the sidebar once this is integrated.
Yes, we'll probably have to keep the dropdown mostly as is, potentially follow up separately. I meant, remove it from the Status & visibility panel so that it isn't duplicated at least. |
|
This is cool! :) One quirk I noticed is how the layout jumps when the featured image loads in. Could we display a placeholder with a Spinner to make that a bit less jarring? Also, perhaps not something to do here, but would it make sense to put the permalink in the summary too?
|
|
This could be a good place for the permalink, probably yes. But I'd like to see if we can land this one in the next day or two 😅 Agreed on the jump, I'm currently tinkering with a better thumbnail, let me see what I can do in terms of a base height. |
|
For reference to the conversation around why the featured image is wrapped in a
So it seems that wrapper is intended to help cover the jumping issue. Let me see if that's still the best way, or if there's something else we can do. |
|
Alright, I pushed some tweaks to the thumbnail handling. I ended up keeping the ResponsiveWrapper, as it did come with the benefit of not oversizing a small image, but still giving a max-width to a large image. I also set the desired size to I also fixed the spinner position:
The issue that Jay reported is one I could use advice on how to fix. Essentially this is the markup when the panel is fully loaded: As you can see, the entire So in terms of next steps, here's the todo list:
How does that sound, @ntsekouras? Do you think we could address and land those in the next couple of days? |
Thanks for all the work here @jasmussen. I'll start working on this right now so hopefully we'll be able to land the v1. |
ntsekouras
force-pushed
the
try/post-summary-panel
branch
from
1f86576 to
8f4538d
Compare
The preference for displaying the previous panel is still respected and will show/hide the featured image from `post summary` now.
…e `excerpt panel` from inspector controls
|
I think #38893 intended to fix it, but as it turns out it's non-trivial. |
|
Oh, and may we have a green check on the design side? 😇 |
|
@Mamaduka is there any chance I could get your eyes and instincts on this one? I'd love to land it, ideally soon, as I think there's a big benefit to it. It works well, and if we run into anything, a revert is always an option. |
|
Sure thing 👍 I am adding to my to-do list. I will try to review it today or tomorrow. |
|
Another quick thought on this one; should the excerpt wrapper have a max-height and scroll behaviour for overflows? I know excerpts by nature should be short, but short is relative. |
I think that can be a perfect followup PR perhaps? |
|
The feature looks great. Excellent work 🥇 I noticed a few issues that we can address in follow-up PRs. Upload spinner is displayed after a few seconds, and it's not clear that the Featured Image started uploading. Here's a screencast with Network throttling (Fast 3G); it's also visible with a normal connection. CleanShot.2022-04-05.at.19.28.17.mp4The This CPT only supports the editor field.
|
|
Thanks a ton for the review. I'll let this sit overnight and then see if we can't press the button tomorrow and follow up on all your points. |
|
@ntsekouras would you mind if we land this one and look at followups? I think it's ready, and we'll have some time in beta to follow up on any bugs noted on this thread. |
ntsekouras
added
the
Needs Dev Note
Good points @Mamaduka! I added the checks and in the case of nothing supported, I think it's okay to look into it in a follow up. |
|
Sounds good, Nik. The feature image issue exists on the trunk as well. So it's not related to this PR, but it would be a good improvement. |
This reverts commit ae657f3.
Part of: #39082
This PR intends to add a fist iteration of
post summarypanel in inspector controls. The new post summary panel will includeFeatured Image, post title, excerpt and author. All of the controls exceptpost titlewhich didn't exist before in inspector controls were removed from their previous positions/panels.Notes
post excerpt and featured imagewould live in their ownpaneland their visibility was also controlled by thepreferences -> panelssection. I have not removed the preferences, and we display these conditionally on the new panel, based on these preferences.Panelsinedit-postpackage for the above(excerpt and featured image)? I think not because they are not exposed anywhere as I could see..editorpackage two wrappers have been added. Do you think this can break backwards compatibility by maybe some third party targeting internal elements with selectors or something similar? 🤔