-
Notifications
You must be signed in to change notification settings - Fork 295
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
Truncate the description value to 100 words #492
Conversation
Currently the value of the `description`, `og: description` and `twitter: description` tags includes the whole text of the post. Which means that, on post pages, the text of the post is included *four* times in the HTML document – adding to the page weight needlessly. This commit ensures the description string is truncated to a sane value.
@kemenaran If it can be customized according to the percentage, this should be the ultimate solution: for example, from 10% to 90% of the full text (if the description is defined in the preface), it is recommended to consider it! |
@mattr- would you consider this improvement mergeable in the current state – or do you prefer to make it configurable? |
280 characters feels like a twitter specific limitation. Regardless of that, a blanket truncation at 280 characters doesn't take into account word or sentence endings, and it feels like we should do that before this PR is mergeable. |
@mattr- thanks for your review! I updated the PR to:
I also added tests, and a new documentation section describing the behavior. Does that look reasonable to you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating this! Everything here looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the code style check came up with a few things to fix. Would you mind addressing those?
4efbca3
to
1198ca6
Compare
Yup, I just pushed fixes to the rubocop issues, it should be good now. |
Thanks for tackling this! @jekyllbot: merge +minor |
Great! Thanks for having a look into this, and for the maintenance work. |
@jekyllbot @kemenaran |
You'll need to wait on a plugin release or tell bundler to fetch the code from the |
Currently, on post pages, the value of the
description
,og: description
,twitter: description
andapplication/ld-json
tags represent the whole text of the post.Which means that the text of the post is included five times in the HTML document – adding to the page weight needlessly.
This commit ensures the description string is truncated to a sane value.