Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start with featured image in media placeholder #41722

Merged
merged 4 commits into from
Jul 6, 2022

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented Jun 14, 2022

What?

This PR allows supporting media blocks to offer users the option to start
with featured image from the placeholder state.

Why?

To avoid having to pick another media to only then get the option and
because the featured image is now hidden behind the "Add media" block
control in the media replace flow.

How?

The PR adds a button to the media placeholder which only shows up if a
handler is provided for clicking it. It is also implemented in the cover
block because this block has a handler.

Testing Instructions

  1. Make a post
  2. Add a cover block
  3. You should see the new "Use featured image" button
  4. Click this button
  5. Set a featured image for the post
  6. Observe the block displays it

Screenshots or screencast

featured-image-placeholder.mp4
@draganescu draganescu added [Type] Enhancement A suggestion for improvement. [Feature] Media Anything that impacts the experience of managing media [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Jun 14, 2022
@github-actions
Copy link

github-actions bot commented Jun 14, 2022

Size Change: +7.09 kB (+1%)

Total Size: 1.25 MB

Filename Size Change
build/annotations/index.min.js 2.76 kB +24 B (+1%)
build/block-editor/index.min.js 152 kB +934 B (+1%)
build/block-editor/style.css 14.5 kB -1 B (0%)
build/block-library/blocks/button/style-rtl.css 543 B +29 B (+6%) 🔍
build/block-library/blocks/button/style.css 543 B +29 B (+6%) 🔍
build/block-library/blocks/comment-template/style-rtl.css 187 B +60 B (+47%) 🚨
build/block-library/blocks/comment-template/style.css 185 B +58 B (+46%) 🚨
build/block-library/blocks/file/style-rtl.css 253 B +1 B (0%)
build/block-library/blocks/file/style.css 254 B +1 B (0%)
build/block-library/blocks/navigation-submenu/view.min.js 402 B +27 B (+7%) 🔍
build/block-library/blocks/navigation/style-rtl.css 1.96 kB +24 B (+1%)
build/block-library/blocks/navigation/style.css 1.95 kB +22 B (+1%)
build/block-library/blocks/navigation/view.min.js 423 B +30 B (+8%) 🔍
build/block-library/blocks/post-comments/style-rtl.css 632 B +4 B (+1%)
build/block-library/blocks/post-comments/style.css 630 B +2 B (0%)
build/block-library/blocks/post-template/style-rtl.css 282 B -41 B (-13%) 👏
build/block-library/blocks/post-template/style.css 282 B -41 B (-13%) 👏
build/block-library/blocks/search/style-rtl.css 385 B -17 B (-4%)
build/block-library/blocks/search/style.css 386 B -17 B (-4%)
build/block-library/blocks/search/theme-rtl.css 114 B +50 B (+78%) 🆘
build/block-library/blocks/search/theme.css 114 B +50 B (+78%) 🆘
build/block-library/index.min.js 183 kB +273 B (0%)
build/block-library/style-rtl.css 11.5 kB +52 B (0%)
build/block-library/style.css 11.5 kB +51 B (0%)
build/block-library/theme-rtl.css 695 B +18 B (+3%)
build/block-library/theme.css 700 B +18 B (+3%)
build/blocks/index.min.js 47.1 kB +96 B (0%)
build/components/index.min.js 230 kB +2.13 kB (+1%)
build/components/style-rtl.css 14 kB -13 B (0%)
build/components/style.css 14 kB -13 B (0%)
build/core-data/index.min.js 14.7 kB -32 B (0%)
build/customize-widgets/index.min.js 11.2 kB +40 B (0%)
build/edit-post/index.min.js 30.4 kB +61 B (0%)
build/edit-post/style-rtl.css 7.04 kB -35 B (0%)
build/edit-post/style.css 7.04 kB -33 B (0%)
build/edit-site/index.min.js 52 kB +2 kB (+4%)
build/edit-site/style-rtl.css 8.28 kB +167 B (+2%)
build/edit-site/style.css 8.26 kB +170 B (+2%)
build/edit-widgets/index.min.js 16.5 kB +72 B (0%)
build/edit-widgets/style-rtl.css 4.36 kB -26 B (-1%)
build/edit-widgets/style.css 4.36 kB -25 B (-1%)
build/editor/index.min.js 39.3 kB +653 B (+2%)
build/editor/style-rtl.css 3.67 kB +45 B (+1%)
build/editor/style.css 3.67 kB +42 B (+1%)
build/element/index.min.js 4.27 kB +3 B (0%)
build/format-library/index.min.js 6.75 kB +152 B (+2%)
build/keyboard-shortcuts/index.min.js 1.78 kB -11 B (-1%)
build/notices/index.min.js 953 B +8 B (+1%)
build/rich-text/index.min.js 11.1 kB +13 B (0%)
build/shortcode/index.min.js 1.53 kB +11 B (+1%)
build/token-list/index.min.js 644 B -18 B (-3%)
build/wordcount/index.min.js 1.06 kB -6 B (-1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 982 B
build/api-fetch/index.min.js 2.26 kB
build/autop/index.min.js 2.14 kB
build/blob/index.min.js 475 B
build/block-directory/index.min.js 6.58 kB
build/block-directory/style-rtl.css 990 B
build/block-directory/style.css 991 B
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 14.5 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 103 B
build/block-library/blocks/audio/style.css 103 B
build/block-library/blocks/audio/theme-rtl.css 110 B
build/block-library/blocks/audio/theme.css 110 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 441 B
build/block-library/blocks/button/editor.css 441 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 95 B
build/block-library/blocks/comments/editor.css 95 B
build/block-library/blocks/cover/editor-rtl.css 615 B
build/block-library/blocks/cover/editor.css 616 B
build/block-library/blocks/cover/style-rtl.css 1.55 kB
build/block-library/blocks/cover/style.css 1.55 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 110 B
build/block-library/blocks/embed/theme.css 110 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/view.min.js 346 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 948 B
build/block-library/blocks/gallery/editor.css 950 B
build/block-library/blocks/gallery/style-rtl.css 1.5 kB
build/block-library/blocks/gallery/style.css 1.49 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 333 B
build/block-library/blocks/group/editor.css 333 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 327 B
build/block-library/blocks/html/editor.css 329 B
build/block-library/blocks/image/editor-rtl.css 738 B
build/block-library/blocks/image/editor.css 737 B
build/block-library/blocks/image/style-rtl.css 524 B
build/block-library/blocks/image/style.css 530 B
build/block-library/blocks/image/theme-rtl.css 110 B
build/block-library/blocks/image/theme.css 110 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 463 B
build/block-library/blocks/latest-posts/style.css 462 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 705 B
build/block-library/blocks/navigation-link/editor.css 703 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/view-modal.min.js 2.78 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 495 B
build/block-library/blocks/post-comments-form/style.css 495 B
build/block-library/blocks/post-comments/editor-rtl.css 77 B
build/block-library/blocks/post-comments/editor.css 77 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 605 B
build/block-library/blocks/post-featured-image/editor.css 605 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 369 B
build/block-library/blocks/query/editor.css 369 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 233 B
build/block-library/blocks/separator/style.css 233 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 464 B
build/block-library/blocks/shortcode/editor.css 464 B
build/block-library/blocks/site-logo/editor-rtl.css 708 B
build/block-library/blocks/site-logo/editor.css 708 B
build/block-library/blocks/site-logo/style-rtl.css 192 B
build/block-library/blocks/site-logo/style.css 192 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.37 kB
build/block-library/blocks/social-links/style.css 1.36 kB
build/block-library/blocks/spacer/editor-rtl.css 322 B
build/block-library/blocks/spacer/editor.css 322 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 494 B
build/block-library/blocks/table/editor.css 494 B
build/block-library/blocks/table/style-rtl.css 611 B
build/block-library/blocks/table/style.css 609 B
build/block-library/blocks/table/theme-rtl.css 175 B
build/block-library/blocks/table/theme.css 175 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 149 B
build/block-library/blocks/template-part/editor.css 149 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 561 B
build/block-library/blocks/video/editor.css 563 B
build/block-library/blocks/video/style-rtl.css 159 B
build/block-library/blocks/video/style.css 159 B
build/block-library/blocks/video/theme-rtl.css 110 B
build/block-library/blocks/video/theme.css 110 B
build/block-library/common-rtl.css 987 B
build/block-library/common.css 984 B
build/block-library/editor-rtl.css 10.2 kB
build/block-library/editor.css 10.2 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-serialization-default-parser/index.min.js 1.11 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/compose/index.min.js 11.7 kB
build/customize-widgets/style-rtl.css 1.4 kB
build/customize-widgets/style.css 1.4 kB
build/data-controls/index.min.js 653 B
build/data/index.min.js 7.95 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 507 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.65 kB
build/edit-navigation/index.min.js 16 kB
build/edit-navigation/style-rtl.css 4.03 kB
build/edit-navigation/style.css 4.04 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/escape-html/index.min.js 537 B
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.64 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.77 kB
build/is-shallow-equal/index.min.js 527 B
build/keycodes/index.min.js 1.38 kB
build/list-reusable-blocks/index.min.js 1.74 kB
build/list-reusable-blocks/style-rtl.css 835 B
build/list-reusable-blocks/style.css 835 B
build/media-utils/index.min.js 2.9 kB
build/nux/index.min.js 2.05 kB
build/nux/style-rtl.css 732 B
build/nux/style.css 728 B
build/plugins/index.min.js 1.94 kB
build/preferences-persistence/index.min.js 2.22 kB
build/preferences/index.min.js 1.3 kB
build/primitives/index.min.js 933 B
build/priority-queue/index.min.js 612 B
build/react-i18n/index.min.js 696 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.69 kB
build/reusable-blocks/index.min.js 2.22 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/server-side-render/index.min.js 1.61 kB
build/url/index.min.js 3.61 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.19 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB

compressed-size-action

@draganescu draganescu requested review from scruffian and javierarce and removed request for ajitbohra and ellatrix June 15, 2022 05:45
@draganescu draganescu force-pushed the add/featured-image-to-media-placeholder branch 2 times, most recently from d491ffd to e91f372 Compare June 15, 2022 09:26
@@ -71,6 +71,7 @@ export function MediaPlaceholder( {
onSelect,
onCancel,
onSelectURL,
onToggleFeaturedImage,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this name. Would onSelectFeaturedImage be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not, because we're not selecting, we're toggling the use of the featured image as a source.

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One nitpick about naming. I can see an opportunity to simplify the code but I'll open an PR for that.

@scruffian
Copy link
Contributor

Here's the refactoring idea: #41742

@javierarce
Copy link
Contributor

This works fine, but it is strange that you can select the option if there isn't a featured image set. This is because the placeholder implies that the post contains a featured image, but that is not true. Also, when you add a cover block this way, you end up inside a paragraph block, which adds more confusion.

A better approach would be only to show the button in an enabled state when the post contains a featured image. If that's not the case, we show a disabled button. This way, it will be evident to users if they have or don't have a featured image set. And in case they don't, they'll know that the cover block supports linking its media to the featured image.

@draganescu
Copy link
Contributor Author

draganescu commented Jun 15, 2022

it is strange that you can select the option if there isn't a featured image set.

@javierarce this reasoning would also apply to the toggle in the add media dropdown in the block toolbar. In the context of a post it may be weird but in the context of template building you don't know if there is a featured image. To reconcile we have that placeholder illustration (the featured image block contains it as well).

In 4bd4761 I added your suggestion, but I don't think the disabled state is helpful. In fact I would always rather to end up with a placeholder than to have a disabled button.

@javierarce
Copy link
Contributor

In 4bd4761 I added your suggestion, but I don't think the disabled state is helpful.

Thanks for adding my suggestion. I think this behavior makes more sense than the initial one, but let's get more eyes on this @WordPress/gutenberg-design.

Something that we should address (with or without this disabled state) is the design of this placeholder, it's getting a bit messy. I'll open a new issue for this once we settle this discussion.

In fact I would always rather to end up with a placeholder than to have a disabled button.

What do you mean by this? 🤔

@javierarce javierarce added the Needs Design Feedback Needs general design feedback. label Jun 21, 2022
@jameskoster
Copy link
Contributor

Hah, I personally find disabled states like this frustrating because it isn't always obvious why something is disabled. If we're going to conditionally disable it I'd prefer to hide it from the UI altogether.

That said, I find the placeholder to be satisfactory as a fallback, but when you elect to use the featured image (in the post editor context) shouldn't we go ahead and open the media library so that you can select/upload a featured image immediately?

@javierarce
Copy link
Contributor

Hah, I personally find disabled states like this frustrating because it isn't always obvious why something is disabled. If we're going to conditionally disable it I'd prefer to hide it from the UI altogether.

I see your point… ok, maybe hiding it completely would be a better approach.

That said, I find the placeholder to be satisfactory as a fallback, but when you elect to use the featured image (in the post editor context) shouldn't we go ahead and open the media library so that you can select/upload a featured image immediately?

I'm not sure what you mean by this. When would we open the media library exactly?

@jameskoster
Copy link
Contributor

I'm not sure what you mean by this. When would we open the media library exactly?

When you select that you want to use the featured image (when a featured image is not already set of course). I dug up the original comment: #40156 (comment)

@javierarce
Copy link
Contributor

Oh, that solution is nice and would reduce the confusion of the disabled/hidden states… but should we have the same text in both cases?

@jameskoster
Copy link
Contributor

but should we have the same text in both cases?

Sorry, which cases? 😅

@javierarce
Copy link
Contributor

When there's a featured image set and when there isn't.

  • If there's one: "Use featured image"
  • If there's none: "Set and use featured image" (?)
@jameskoster
Copy link
Contributor

Ah I see. Yeah that could be nice. Another option would be to leave the label the same but adjust the media library modal title to say something like "Select or upload media to use as featured image".

@draganescu
Copy link
Contributor Author

@javierarce @jameskoster I am unsure if this PR is "blocked" or we can merge and iterate :D The addition of the button itself is a nice quality of life addition. Do you think merging this without the deep integration to the featured media feature is a no go?

@draganescu draganescu force-pushed the add/featured-image-to-media-placeholder branch from 4bd4761 to 8258f1f Compare June 21, 2022 14:12
@jameskoster
Copy link
Contributor

I lean towards using the placeholder graphic. I find the current state to be very confusing:

Screenshot 2022-06-22 at 10 01 38

@paaljoachim
Copy link
Contributor

Adding a featured image link to the Cover block placeholder is helpful. It would be even more helpful to be able to also set the featured image at the same time if one has not been set.

@draganescu draganescu merged commit 6cdd6ae into trunk Jul 6, 2022
@draganescu draganescu deleted the add/featured-image-to-media-placeholder branch July 6, 2022 14:12
@github-actions github-actions bot added this to the Gutenberg 13.7 milestone Jul 6, 2022
@draganescu
Copy link
Contributor Author

@paaljoachim yes that is true.

I am thinking about how this could work since introducing this "set featured image functionality" to the cover block would change the fact that we toggle the binding off when other media is selected.

The interaction in the post featured image can't be used since the "chip" is where the cover block content is.

... still thinking 😁

@annezazu annezazu added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jul 22, 2022
@annezazu
Copy link
Contributor

Marked three related PRs as needing backports to 6.0.2 including this PR, #41476, and #41460. Marking them as backports because, currently, with 6.0.1, the "set as featured image" option in the toolbar leads to a confusing UX/dead end where you can't add anything further until you select either an image from the media library or a color. Here's a quick video:

featured.image.mov

Noting this here for this specific PR since I think it does the most to resolve the current pain points :)

@gziolo
Copy link
Member

gziolo commented Aug 23, 2022

I removed this PR from the WordPress 6.0.2 backporting plan. Unfortunately, these changes depend on a larger refactoring that is rather risky to bring in the bug fix release. It also doesn’t apply cleanly, so I don’t feel comfortable patching the branch for WordPress core manually.

@gziolo gziolo removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 23, 2022
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Sep 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Media Anything that impacts the experience of managing media Needs Design Feedback Needs general design feedback. Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement.
8 participants