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

@wordpress/editor: Add estimated time to read to table of contents in editor #41611

Merged
merged 5 commits into from
Jul 6, 2022

Conversation

opr
Copy link
Contributor

@opr opr commented Jun 8, 2022

What?

Adds an estimated time to read to the table of contents in the post editor.

Why?

It would be a useful tool for editors to know how long their article will take (the average) reader to read.

Closes #38593.

How?

I took an average of the reading rate (words per minute, but characters per minute for Chinese) from this source: https://irisreading.com/average-reading-speed-in-various-languages/. A lot of other sources cite 200 wpm is the average reading rate but I think they didn't account for other languages. Using the average of 189 should get us a pretty good estimate.

Using this, I created a TimeToRead function, which uses count from @wordpress/wordcount to calculate how many minutes to display.

The number is styled larger than the text. Please guide me on styling if we should do something different.

Testing Instructions

  1. Open a post or page and enter some text.
  2. Open the table of contents panel (the i above the editor).
  3. See the time to read presented at the end of the list.
  4. Change the word count in your post, ensure the time to read increases appropriately. For every 189 words you add the time should increase by one minute.

Screenshots or screencast

image

Additional thoughts

If #41598 gets merged then we could possibly use humanTimeDiff to get the locale specific time between now and the estimated minutes to read. This displays time differences including from a few seconds, x minutes, x hours, x days, x months and x years.

The challenge here would be identifying how to split the resulting string into its locale-correct constituent parts (number of units of time, and string describing the unit of time) and styling the number and string differently. We also need to consider the case of the text only option, where the words are such that the post will take a single unit of time to read (i.e. a minute) and how we would style a minute where no number is present vs 2 minutes where the number being larger than the text looks alright.

return (
<span className="time-to-read">
<span className="table-of-contents__number">
{ minutesToRead }{ ' ' }
Copy link
Contributor Author

@opr opr Jun 8, 2022

Choose a reason for hiding this comment

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

Is there a better way to leave a space between the number and the text, is CSS more appropriate for this?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe using sprintf from @wordpress/i18n would be the way to go. It would also cover those languages that would put the word minutes after the number.

Copy link
Member

@gziolo gziolo Jun 15, 2022

Choose a reason for hiding this comment

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

There is also createInterpolateElement from @wordpress/element that can cover using React components:

https://github.com/WordPress/gutenberg/tree/trunk/packages/element#createinterpolateelement

Example usage:

text = createInterpolateElement(
sprintf(
/* translators: %s: search term. */
__( 'Create: <mark>%s</mark>' ),
searchTerm
),
{ mark: <mark /> }
);

@alexstine
Copy link
Contributor

@opr I'd like to accessibility review this PR but have no idea where the table of contents is. What is the aria-label on the button?

Thanks.

@opr
Copy link
Contributor Author

opr commented Jun 9, 2022

Hi @alexstine thank you - sure the aria-label for the button is Details - it's at the top above the post. It's in the Document tools group of icons.

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

@opr Accessibility is good and I think the spacing reflects how other items are listed. I only checked this for keyboard navigation and general usability/code quality, I am unable to do visual testing. No idea how it looks. A sighted review would probably be good for this.

@gziolo gziolo added [Package] Editor /packages/editor [Type] Enhancement A suggestion for improvement. labels Jun 15, 2022
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Excellent addition. I left some minor feedback, but overall this looks great 💯

return (
<span className="time-to-read">
<span className="table-of-contents__number">
{ minutesToRead }{ ' ' }
Copy link
Member

Choose a reason for hiding this comment

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

Maybe using sprintf from @wordpress/i18n would be the way to go. It would also cover those languages that would put the word minutes after the number.

* Do not translate into your own language.
*/
const wordCountType = _x( 'words', 'Word count type. Do not translate!' );
const minutesToRead = Math.round(
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a special case for minutes to read less than 1 (or 0.5)? In that case, we could display: Less than a minute. Otherwise, we might see 0 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gziolo I went with < 1 minute because without there being a number there it looks kind of weird.

image

Compared to:

image

@opr
Copy link
Contributor Author

opr commented Jun 17, 2022

@gziolo awesome thanks for the reminder about createInterpolateElement this was really helpful!

@opr opr requested a review from gziolo June 17, 2022 14:23
@gziolo gziolo added the Needs Design Feedback Needs general design feedback. label Jun 28, 2022
@gziolo
Copy link
Member

gziolo commented Jun 28, 2022

Code-wise everything is good. I would appreciate design feedback for this one. I wonder whether the distribution of item in the popover should be a bit different, for example:

Screenshot 2022-06-28 at 13 44 14

@gziolo
Copy link
Member

gziolo commented Jun 28, 2022

By the way, @richtabor created a WordPress plugin with a Post Reading Time block that is available at https://github.com/richtabor/post-reading-time-block. It could be a good addition to the core as well.

@jameskoster
Copy link
Contributor

I wonder whether the distribution of item in the popover should be a bit different

Your suggestion looks good to me.

@opr
Copy link
Contributor Author

opr commented Jul 2, 2022

@gziolo and @jameskoster thanks for your input. I've made the changes you suggested and re-ordered the panel.

As for the time to read block, I think it would be better to bring that in in a separate PR, do you agree?

@gziolo
Copy link
Member

gziolo commented Jul 4, 2022

As for the time to read block, I think it would be better to bring that in in a separate PR, do you agree?

Surely, it's outside of the scope of this PR. We want to keep it small. It's a good opportunity to discuss further work though before we jump into opening issues.

thanks for your input. I've made the changes you suggested and re-ordered the panel.

Looks good to me! I re-trigger one of the CI job to see if the test (unrelated to this PR) passes.

@gziolo gziolo merged commit bc071fc into WordPress:trunk Jul 6, 2022
@gziolo gziolo removed the Needs Design Feedback Needs general design feedback. label Jul 6, 2022
@github-actions github-actions bot added this to the Gutenberg 13.7 milestone Jul 6, 2022
@opr opr deleted the add/time-to-read branch July 11, 2022 08:51
@bph bph mentioned this pull request Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs User Documentation Needs new user documentation [Package] Editor /packages/editor [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Word count and estimated time to read
5 participants