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

feat: add "unlisted" front matter option for blog posts #1396

Merged
merged 2 commits into from
Jun 15, 2019

Conversation

olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Apr 26, 2019

Previously, the blog sidebar listed the most recent blog posts sorted by
their publish date. This commit adds the ability to hide a specific blog
post from the sidebar and the feed of all blog posts by adding the
unlisted: true header option.

Fixes #1393.

Motivation

The motivation for this feature is to make it possible to change dates of published posts without breaking the old URL (which might be widely shared).

Concrete example, we published a blog post two weeks ago that is dated for 2020 (https://scalameta.org/metals/blog/2020/04/12/mercury.html). We have closed three PRs fixing the date (scalameta/metals#683, scalameta/metals#670, scalameta/metals#656) but they have all been closed because changing the date of the post would turn the old URL into a 404.

With the ability to unlist posts, I could unlist the 2020 post, create a new posts dated in 2019 and update the unlisted 2020 post to forward to the new 2019 post.

I'm open for alternative solutions to fix that particular problem. I think unlisting is a flexible solution that could be useful in other situations as well, for example when writing a draft post.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

I would appreciate help for testing this change since I'm unfamiliar with jest (and JavaScript in general 😅 ). I created a blog post in __test__/__fixtures__ using unlisted: true and ran yarn test and everything passed but I expected some snapshot tests to fail.

I tested the change manually on our website and it seems to work as expected:

2019-04-26 11 04 57

Observe that uncommenting unlisted: true makes the post disappear from both the sidebar and the list of all blog posts while the post is still published under the unchanged URL.

Related PRs

I'm not aware of any.

@olafurpg olafurpg requested a review from endiliey as a code owner April 26, 2019 09:14
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 26, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 26, 2019

Deploy preview for docusaurus-2 ready!

Built with commit 031c2cb

https://deploy-preview-1396--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 26, 2019

Deploy preview for docusaurus-preview ready!

Built with commit 031c2cb

https://deploy-preview-1396--docusaurus-preview.netlify.com

Previously, the blog sidebar listed the most recent blog posts sorted by
their publish date. This commit adds the ability to hide a specific blog
post from the sidebar and the feed of all blog posts by adding the
`unlisted: true` header option.

```md
title: My Draft Post
unlisted: true # Hide from blog sidebar and main blog page feed.
---
```
@@ -10,6 +10,8 @@ const BlogPost = require('./BlogPost.js');
const BlogSidebar = require('./BlogSidebar.js');
const Container = require('./Container.js');
const MetadataBlog = require('./MetadataBlog.js');

const MetadataPublicBlog = MetadataBlog.filter(item => item.unlisted !== true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where would be a good place to put this shared logic between BlogSidebar.js, BlogPageLayout.js and feed.js?

@@ -10,6 +10,8 @@ const BlogPost = require('./BlogPost.js');
const BlogSidebar = require('./BlogSidebar.js');
const Container = require('./Container.js');
const MetadataBlog = require('./MetadataBlog.js');

const MetadataPublicBlog = MetadataBlog.filter(item => item.unlisted !== true);
Copy link

Choose a reason for hiding this comment

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

Suggested change
const MetadataPublicBlog = MetadataBlog.filter(item => item.unlisted !== true);
const MetadataPublicBlog = MetadataBlog.filter(item => !item.unlisted);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

The change looks good in general, but I'd like to suggest that draft: true could be a better naming than unlisted: true. Jekyll calls them drafts too.

Is there currently a way for you to view your unlisted/draft posts without toggling the flag?

In v2 we should definitely support this feature and allow a command line flag or config option on whether to display them during development.

@@ -28,6 +28,9 @@ module.exports = function(type) {

readMetadata.generateMetadataBlog();
const MetadataBlog = require('../core/MetadataBlog.js');
const MetadataPublicBlog = MetadataBlog.filter(
item => item.unlisted !== true,
Copy link
Contributor

Choose a reason for hiding this comment

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

item => !item.unlisted,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@olafurpg
Copy link
Contributor Author

Thank you for the review! I've renamed "unlisted" into "draft" and incorporated the !item.draft refactoring. Note that there is a difference between draft posts in Jekyll and how they are implemented in this PR.

  • Jekyll draft posts are ignored during a non-development build and they do not need a date in the filename
  • draft posts in this PR still require a date in the filename and are published during non-development builds, they just don't appear in the blog sidebar or main blog feed

Is there currently a way for you to view your unlisted/draft posts without toggling the flag?

Not in the current implementation. It would be nice to see draft posts during livereload development but I'm not sure how to implement that.

@gabro
Copy link

gabro commented Apr 29, 2019

About unlisted vs draft. The way I would expect "draft" to behave is not to make the post public, whereas unlisted would be published but not listed in the menu.

To summarize

status visible in menu published
draft no no
unlisted no yes
normal yes yes

For this reason, I think 'unlisted' was the appropriate name for the changes included in this PR.

@olafurpg
Copy link
Contributor Author

Anything I can do to help unblock this issue? All of the review comments from #1396 (review) have been addressed, although I believe it's worth considering using unlisted instead of draft due to the reasons mentioned by @gabro in #1396 (comment)

@endiliey
Copy link
Contributor

cc @yangshun @wgao19

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

@olafurpg Sorry for the late reply. @gabro's comment convinced me that unlisted sounds more appropriate although I'm not leaning strongly towards any particular term. Medium is essentially doing something similar. I'll merge this and make the changes myself since I was the one who asked for the bad change 😅

@yangshun yangshun changed the title Add unlisted header option for blog posts, fixes #1393. feat: add unlisted header option for blog posts Jun 15, 2019
@yangshun yangshun changed the title feat: add unlisted header option for blog posts feat: add "unlisted" front matter option for blog posts Jun 15, 2019
@yangshun yangshun merged commit da3c913 into facebook:master Jun 15, 2019
@olafurpg olafurpg deleted the hide-from-blog-sidebar branch June 15, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to unlist blog post from sidebar
7 participants