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

Allow the time to read block to be inserted multiple times #49253

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Mar 22, 2023

What?

In passing, I noticed that the Time to Read block can't be inserted in a post more than once.

In this PR I'm removing the restriction. I'm not sure if there's a reason I'm missing for keeping multiple: false.

Why?

While it's likely the user would only want the block once, I don't think there's any need to impose the restriction.

How?

Remove the line in the block.json

Testing Instructions

  1. Add a time to read block to a post
  2. Try adding another time to read block, it should work.

Screenshots or screencast

Screen Shot 2023-03-22 at 4 32 24 pm

@talldan talldan added [Type] Enhancement A suggestion for improvement. [Block] Time to Read Affects the Time to Read Block labels Mar 22, 2023
@talldan talldan requested a review from t-hamano March 22, 2023 08:33
@talldan talldan self-assigned this Mar 22, 2023
@github-actions
Copy link

Flaky tests detected in 0883c6a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4487868973
📝 Reported issues:

Copy link
Member

@fabiankaegy fabiankaegy left a comment

Choose a reason for hiding this comment

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

Yeah especially when you want to use the block once in the main post content and then again in a query loop template we need to be able to insert it more than once.

(I still think #38958 would make sense for this at some point :) )

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

LGTM! I can't remember why I set multiple to false when I created this block in #43403, but I see no reason to limit it 😅

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

LGTM!
The PR needs to be rebased because there are currently some conflicts, but other than that it's good to merge 👍

@talldan talldan force-pushed the add/multiple-support-to-time-to-read-block branch from 0883c6a to ac121ca Compare March 27, 2023 06:47
@fabiankaegy fabiankaegy enabled auto-merge (squash) March 27, 2023 06:48
@fabiankaegy fabiankaegy merged commit 521e967 into trunk Mar 27, 2023
@fabiankaegy fabiankaegy deleted the add/multiple-support-to-time-to-read-block branch March 27, 2023 07:53
@github-actions github-actions bot added this to the Gutenberg 15.5 milestone Mar 27, 2023
@talldan
Copy link
Contributor Author

talldan commented Mar 27, 2023

Thanks for the quick reviews ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Time to Read Affects the Time to Read Block Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants