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

Removed the less than 1 min reading time clause #9573

Merged
merged 2 commits into from Apr 18, 2018

Conversation

Projects
None yet
3 participants
@rkalis
Contributor

rkalis commented Apr 16, 2018

closes #9569

  • Removed the less than 1 minute reading time clause, effectively making 1 min the minimum reading time
  • Removed the 'seconds' option for i18n strings, which contained the less than one minute display string
  • Kept the other i18n string options the same
  • Amended and improved tests for new functionality
Removed the less than 1 min reading time clause
closes #9569
- Removed the less than 1 minute reading time clause, effectively making 1 min the minimum reading time
- Removed the 'seconds' option for i18n strings, which contained the less than one minute display string
- Kept the other i18n string options the same
- Amended and improved tests for new functionality
@@ -48,11 +47,9 @@ module.exports = function reading_time(options) {// eslint-disable-line camelcas
readingTimeSeconds += Math.max(i, 3);
}
readingTimeMinutes = Math.round(readingTimeSeconds / 60);
readingTimeMinutes = Math.max(Math.round(readingTimeSeconds / 60), 1);

This comment has been minimized.

@vikaspotluri123

vikaspotluri123 Apr 16, 2018

Contributor

Instead of this, couldn't we just use a <= comparison below? I think it would make readability a little easier

This comment has been minimized.

@rkalis

rkalis Apr 16, 2018

Contributor

Makes sense.

@AileenCGN

AileenCGN approved these changes Apr 18, 2018 edited

Tested successfully, also with translated themes that still use seconds in the {{reading_time}} helper. 👍

@AileenCGN AileenCGN merged commit 2a4d759 into TryGhost:master Apr 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment