Add option to make Post Featured Image a link #25714

ntsekouras · 2020-09-29T18:19:30Z

Description

This PR adds an option to make Post Featured Image block a link. ~~It handles target and rel attributes as well.~~

How has this been tested?

Locally

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-09-29T18:28:49Z

Size Change: -157 B (0%)

Total Size: 1.17 MB

Filename	Size	Change
`build/block-editor/index.js`	129 kB	+12 B (0%)
`build/block-editor/style-rtl.css`	11 kB	-122 B (1%)
`build/block-editor/style.css`	11 kB	-124 B (1%)
`build/block-library/editor-rtl.css`	8.6 kB	+9 B (0%)
`build/block-library/editor.css`	8.6 kB	+9 B (0%)
`build/block-library/index.js`	135 kB	-2 B (0%)
`build/block-library/style-rtl.css`	7.65 kB	+6 B (0%)
`build/block-library/style.css`	7.65 kB	+6 B (0%)
`build/components/style-rtl.css`	15.4 kB	-7 B (0%)
`build/components/style.css`	15.4 kB	-7 B (0%)
`build/edit-navigation/index.js`	10.7 kB	-5 B (0%)
`build/edit-site/index.js`	20.4 kB	+33 B (0%)
`build/edit-site/style-rtl.css`	3.78 kB	+18 B (0%)
`build/edit-site/style.css`	3.78 kB	+17 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.52 kB	0 B
`build/api-fetch/index.js`	3.35 kB	0 B
`build/autop/index.js`	2.72 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	8.61 kB	0 B
`build/block-directory/style-rtl.css`	943 B	0 B
`build/block-directory/style.css`	942 B	0 B
`build/block-library/theme-rtl.css`	741 B	0 B
`build/block-library/theme.css`	741 B	0 B
`build/block-serialization-default-parser/index.js`	1.78 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	47.5 kB	0 B
`build/components/index.js`	168 kB	0 B
`build/compose/index.js`	9.42 kB	0 B
`build/core-data/index.js`	12 kB	0 B
`build/data-controls/index.js`	1.27 kB	0 B
`build/data/index.js`	8.42 kB	0 B
`build/date/index.js`	31.9 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	4.42 kB	0 B
`build/edit-navigation/style-rtl.css`	868 B	0 B
`build/edit-navigation/style.css`	871 B	0 B
`build/edit-post/index.js`	306 kB	0 B
`build/edit-post/style-rtl.css`	6.25 kB	0 B
`build/edit-post/style.css`	6.24 kB	0 B
`build/edit-widgets/index.js`	18.7 kB	0 B
`build/edit-widgets/style-rtl.css`	3.03 kB	0 B
`build/edit-widgets/style.css`	3.03 kB	0 B
`build/editor/editor-styles-rtl.css`	492 B	0 B
`build/editor/editor-styles.css`	493 B	0 B
`build/editor/index.js`	45.5 kB	0 B
`build/editor/style-rtl.css`	3.83 kB	0 B
`build/editor/style.css`	3.82 kB	0 B
`build/element/index.js`	4.44 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.49 kB	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	1.74 kB	0 B
`build/html-entities/index.js`	621 B	0 B
`build/i18n/index.js`	3.55 kB	0 B
`build/is-shallow-equal/index.js`	709 B	0 B
`build/keyboard-shortcuts/index.js`	2.39 kB	0 B
`build/keycodes/index.js`	1.85 kB	0 B
`build/list-reusable-blocks/index.js`	3.02 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/media-utils/index.js`	5.12 kB	0 B
`build/notices/index.js`	1.69 kB	0 B
`build/nux/index.js`	3.27 kB	0 B
`build/nux/style-rtl.css`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/plugins/index.js`	2.44 kB	0 B
`build/primitives/index.js`	1.34 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	13 kB	0 B
`build/server-side-render/index.js`	2.6 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.24 kB	0 B
`build/url/index.js`	4.06 kB	0 B
`build/viewport/index.js`	1.74 kB	0 B
`build/warning/index.js`	1.13 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

aristath · 2020-09-30T05:11:07Z

packages/block-library/src/post-featured-image/block.json

+		},
+		"linkTarget": {
+			"type": "string",
+			"default": "_blank"


Why are we defaulting to _blank?

Hm.. I guess it was natural to me as I've been expecting and prefer that approach, but after your comment and some research seems to be incorrect. Thanks for pointing this out Ari.

I have pushed the changes to default and changed PostTitle default as well.

aristath

Thank you @ntsekouras! LGTM 👍

jasmussen · 2020-09-30T07:22:04Z

Nice PR. I was about to echo Ari that defaulting to same window is always best :)

This is what I see:

This is all in the json configuration, correct? No UI?

ntsekouras · 2020-09-30T07:45:21Z

Hey @jasmussen!

This is all in the json configuration, correct? No UI?

Actually no. There are two cases where a placeholder is shown.

When there is no query context (like your use case from the screenshot in default front-page with show latest posts). Wasn't that it?
When there is context but no Featured Image is set.

I pushed a new. commit to show in both cases the chip we've been discussing in the other PR. Can you take it for a spin again? :)

You can test this with Query block as well.

jasmussen · 2020-09-30T08:34:31Z

Nice, this is closer. I see this when no featured image:

And this when I do:

The link UI is rather different than the Image link UI, though:

This presents a few questions:

What does the featured image link to? The post? The image itself? Can I write my own custom link?
Since the featured image is entirely a template part, you should be able to edit and configure it even without the right content. I.e. I need to be able to configure that a featured image links, without having to write a post with a featured image first.
If the link indeed does link to the post itself the toggle should be rephrased to say "Link to post". (or post type?). And also — should a link to a post ever open in a new tab, and what would be the use case of the rel?

aristath · 2020-09-30T08:37:28Z

Should it even have the "Open in new link" option? Since the image will always link to the same thing and is not editable, I don't think we'll ever need to open in a new tab... 🤔

ntsekouras · 2020-09-30T08:51:21Z

Great questions!

What does the featured image link to? The post? The image itself? Can I write my own custom link?

It links to the entity and not the image. I think it might be considered to link to the image in a follow up PR. It doesn't make much sense to me to allow a custom link as this can be achieved with the Image block and PostFeaturedImage is much more specific to the current post.

Since the featured image is entirely a template part, you should be able to edit and configure it even without the right content. I.e. I need to be able to configure that a featured image links, without having to write a post with a featured image first.

That makes sense yes. Especially in Query block usage.

If the link indeed does link to the post itself the toggle should be rephrased to say "Link to post". (or post type?). And also — should a link to a post ever open in a new tab, and what would be the use case of the rel?

Should it even have the "Open in new link" option? Since the image will always link to the same thing and is not editable, I don't think we'll ever need to open in a new tab..

The label could become more specific like Make featured image a link to {entity}. Regarding new tab and rel, I think it's up for the user to decide if wants to enable and what rel to add there..

I'll make the above changes.

jasmussen · 2020-09-30T08:57:35Z

To be clear, what I'm advocating for is to start with fewer features. It's always easier to add features once it becomes evident that they are necessary, whereas it is often hard to remove features when they turn out to be mostly unnecessary.

In that vein, I would recommend:

Link toggle working for both placeholder and empty state
No external link toggle, no rel feature until we know we need it
Simpler phrasing of the toggle.

The label could become more specific like Make featured image a link to {entity}.

Nice. Can we shorten it and be more action'y? Such as: Link to {entity}? Keep in mind, there's this right above providing ample context:

ntsekouras · 2020-09-30T09:25:47Z

The changes I made are:

remove target and rel.
Changed the label to also mention the post type of the context it lives.
Made the option show even if no featured image is set. This doesn't apply when there is no context like I mentioned above. I'm not sure how to handle this yet, but I think it's related with a decision that affects all PostBlocks.
Removed the link entirely from edit as it visually changes nothing and it makes it hard to select the block, if the image is wide.

* Add option to link Post Featured Image * make default to open in same tabs * show chip in both cases for placeholder * address review feedback * fix whitespace

* Don't use _blank unless needed * now expects _self * done in #25714 * Revert category changes

Add option to link Post Featured Image

ad382a4

ntsekouras added [Feature] Full Site Editing [Type] Feature New feature to highlight in changelogs. [Block] Post Featured Image Affects the Post Featured Image Block labels Sep 29, 2020

ntsekouras requested review from mcsf, aristath and jasmussen September 29, 2020 18:19

ntsekouras requested review from ajitbohra, chrisvanpatten, nerrad, ntwb, Soean, talldan and youknowriad as code owners September 29, 2020 18:19

ntsekouras self-assigned this Sep 29, 2020

ntsekouras requested a review from mapk September 29, 2020 18:22

aristath reviewed Sep 30, 2020

View reviewed changes

make default to open in same tabs

4be915b

aristath approved these changes Sep 30, 2020

View reviewed changes

show chip in both cases for placeholder

f5c3b65

ntsekouras mentioned this pull request Sep 30, 2020

Don't use target="_blank" unless needed #25730

Merged

6 tasks

aristath added a commit that referenced this pull request Sep 30, 2020

done in #25714

0cdaac8

address review feedback

c28b7d2

fix whitespace

8be3f80

ntsekouras merged commit 398d206 into master Sep 30, 2020

ntsekouras deleted the add/featured-image-option-to-link branch September 30, 2020 09:53

github-actions bot added this to the Gutenberg 9.2 milestone Sep 30, 2020

aristath added a commit that referenced this pull request Nov 17, 2020

Don't use target="_blank" unless needed (#25730)

270f6eb

* Don't use _blank unless needed * now expects _self * done in #25714 * Revert category changes

Mamaduka mentioned this pull request Oct 7, 2021

Featured Image block: Add an option to link to the post #33602

Closed

felixarntz mentioned this pull request Nov 16, 2021

Fix layout shift in core/post-featured-image block with isLink enabled #36552

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to make Post Featured Image a link #25714

Add option to make Post Featured Image a link #25714

ntsekouras commented Sep 29, 2020 •

edited by mcsf

github-actions bot commented Sep 29, 2020 •

edited

aristath Sep 30, 2020

ntsekouras Sep 30, 2020

aristath left a comment

jasmussen commented Sep 30, 2020

ntsekouras commented Sep 30, 2020 •

edited

jasmussen commented Sep 30, 2020

aristath commented Sep 30, 2020 •

edited

ntsekouras commented Sep 30, 2020

jasmussen commented Sep 30, 2020

ntsekouras commented Sep 30, 2020

Add option to make Post Featured Image a link #25714

Add option to make Post Featured Image a link #25714

Conversation

ntsekouras commented Sep 29, 2020 • edited by mcsf

Description

How has this been tested?

Checklist:

github-actions bot commented Sep 29, 2020 • edited

aristath Sep 30, 2020

Choose a reason for hiding this comment

ntsekouras Sep 30, 2020

Choose a reason for hiding this comment

aristath left a comment

Choose a reason for hiding this comment

jasmussen commented Sep 30, 2020

ntsekouras commented Sep 30, 2020 • edited

jasmussen commented Sep 30, 2020

aristath commented Sep 30, 2020 • edited

ntsekouras commented Sep 30, 2020

jasmussen commented Sep 30, 2020

ntsekouras commented Sep 30, 2020

ntsekouras commented Sep 29, 2020 •

edited by mcsf

github-actions bot commented Sep 29, 2020 •

edited

ntsekouras commented Sep 30, 2020 •

edited

aristath commented Sep 30, 2020 •

edited