Try: Let Featured Image block inherit dimensions, look like a placeholder #36517
Conversation
jasmussen
added
the
[Block] Post Featured Image
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 60 60" | ||
| preserveAspectRatio="xMidYMid slice" // @todo: "slice" matches the "cover" behavior, "meet" could be used for "container" and "fill" values. |
There was a problem hiding this comment.
Left a todo here, as the meet property of this SVG attribute would let us actually resize the illustration same as if it was an image set to "Contain".
| height: auto; | ||
| display: block; | ||
| // Give the featured image placeholder the appearance of a literal image placeholder. | ||
| // @todo: this CSS is similar to that of the Site Logo. That makes it an opportunity |
There was a problem hiding this comment.
Most of this code is copied verbatim from #35397, and suggests there is an opportunity to create a new component, such as InheritingPlaceholder (it'd need a better name), which comes with less content inside, usually just a single upload/media library button, minimal markup, and intentionally lets theme styles bleed in and affect it.
It doesn't necessarily have to happen as part of this PR, but it seems worth tracking if this PR goes any further.
|
|
||
| .editor-styles-wrapper { | ||
| .post-featured-image_placeholder { |
There was a problem hiding this comment.
These styles were actually dead, and didn't target anything.
| // Provide a minimum size for the placeholder when resized. | ||
| // Note, this should be as small as we can afford it, and exists only | ||
| // to ensure there's room for the upload button. | ||
| &[style*="height"] .components-placeholder { |
There was a problem hiding this comment.
Using this style we can override the default placeholder min-height with an explicit height as soon as one is applied to the container.
|
Size Change: +6.04 kB (+1%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
|
Fixes #36136. |
priethor
added
the
Backport to WP Beta/RC
There was a problem hiding this comment.
Thanks for addressing this, @jasmussen. Works as expected!
In comparison to the Site Logo block, the Post Featured Image doesn't have a Reset option in the Replace toolbar menu. Should it have it, too, for consistency? It would be redundant with the inspector's Post tab, but as a user, I expected to reset the image through the toolbar given I originally uploaded the image directly through the Post Featured Image block's toolbar and didn't use the inspector.
|
Thank you for fast review! I'm at my evening here, so I'll press the green button tomorrow morning to be present. That also affords time for an additional sanity check. Otherwise if it's urgent, feel free to merge and I'll follow up on anything.
I could swear there was a good reason for why we ended up not doing this, though the precise reason escapes me at this point. I might also be misremembering and thinking of a different block. 🤔 |
|
Is it too much of a stretch to allow duotone to pass its color through to the border when in placeholder state? |
|
I'll take a look! |
|
Just as a quick and hacky smoketest it works decently on the border as it picks up the dark/shadow color of the defined duotone. But if we apply the filter in a simple manner, it also applies to the button inside:
The filter can be applied to the dashed border and illustration separately:
Both solutions would require a little bit of dev work. By default the duotone simply targets the In order to target the placeholder inside, we'd need for that numbered filter to also target either |
|
Hi, @priethor Here's a recent discussion about the "Reset" button - #35417 (comment). Happy to work on follow-up PR to bring the feature back. |
|
In porting over the code from the Site Logo block, I had forgotten to port over the change from using a Notice to using a Snackbar. In my latest commit, I ported over that part: @priethor if you have time, can you look at the code just one more time before I press the green button? The code was originally written by Kerry, so it should be solid. Just want to make sure I ported it over correctly. It works correctly for me. |
|
Much better! I'm seeing this in the site editor though:
|
|
Huh, that's curious. I'll take a look in a moment. |
|
One other thing (we may need to address this in a follow-up), I see no way to unset the featured image via the block – I have to go to the post settings in the Inspector. Let me know if you'd like me to open a separate issue. |
|
That keeps coming up :) See #36517 (comment) |
|
Hah, I opened a separate issue: #36566 |
|
@jameskoster can you take it for a spin again? Perhaps with a fresh npm install before you npm run dev? Or maybe clear cache? Just tried again and it's working well in the site editor for me: |
|
No luck :/ It's working fine in the post editor. Which template are you editing? The culprit seems to be lines 32-37 in packages/block-library/src/post-featured-image/edit.js |
|
Good catch. There was a condition to show a placeholder that had no upload button, which I hadn't caught. That should be addressed now, though given the changes I'd appreciate an extra look at the code just to be sure. |
|
Bingo! It's working for me now :) |
|
Cool, I'll see if I can get the tests passing. |
|
Nice! We can continue to improve this, but it's already much more meaningful for template editing for understanding what's going on. Thanks for working on this. |
|
Always a pleasure. |
|
Apologies for missing this one before merging. It's looking WAY better indeed! Kudos @jasmussen Perhaps it's worth exploring a new path where we don't use the mountains symbol which works well in smaller contexts like site icon, but in larger cases it seems to stretch to an odd form.
Notice that it also has a gap on the left (or bottom) side.
We could lean more on an explicit wireframe style with lines, wether with a single diagonal or crossing the container. A bit more abstract that doesn't conflict with potential symbols in the theme itself.
|
|
Nice! Thanks for the mockups. I like the one with the straight diagonal. I'll take a stab. |
noisysocks
removed
the
Backport to WP Beta/RC
…lder (#36517) * Try: Let Featured Image block inherit dimensions, look like a placeholder * Inherit dimensions. * Change notice to snackbar. * Fix edgecase.
|
I created a followup here: #36712 |
Description
The Featured Image placeholder state suffers from not looking anything like the end result. It's a bordered box, and althoug hit can inherit the width as defined in the inspector, it does not inherit the height:
While the bordered box placeholder pattern can be useful for certain blocks like Cover and others, the primary use case of the Featured Image block is to be inserted in site templates, as-is. It will then surface images set as featured by posts. But when no image is set as featured, the placeholder is shown. That makes it more important that the placeholder state looks at least a little bit like the end result.
This PR takes the same approach as was taken for the Site Logo block recently (#35397), and redesigns the placeholder to look like a literal placeholder image:
The dashed line inherits the text color so it's always visible regardless of background color. It inherits width and height, as well as border-radius (even if that can't yet be set using the inspector). The end result is a visually more clear purpose when seen in the Site editor:
How has this been tested?
Insert a featured image block. Verify it works as it did before. Try adding an image using the upload button. Try changing dimensions and background position of the block, both with and without an image set.
Checklist:
*.native.jsfiles for terms that need renaming or removal).