Skip to content

Docs: clarification about page title #142

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

Merged
merged 8 commits into from
Jul 17, 2017
Merged

Conversation

antgel
Copy link
Contributor

@antgel antgel commented Jul 13, 2017

I had to dig into the source code to find out why my page links weren't automagically generated - this patch will help others in the same boat.

README.md Outdated
@@ -35,7 +35,7 @@ Refers to files within the `_layouts` directory, that define the markup for your

- `default.html` — The base layout that lays the foundation for subsequent layouts. The derived layouts inject their contents into this file at the line that says ` {{ content }} ` and are linked to this file via [FrontMatter](https://jekyllrb.com/docs/frontmatter/) declaration `layout: default`.
- `home.html` — The layout for your landing-page / home-page / index-page.
- `page.html` — The layout for your documents that contain FrontMatter, but are not posts.
- `page.html` — The layout for your documents that contain FrontMatter, but are not posts. By default (unlike posts), page FrontMatter requires a `title` attribute, in order for a link to the page to be generated (in `header.html`).
Copy link
Member

Choose a reason for hiding this comment

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

IMO, instead of adding it here, it'd be better next to _includes - header.html with the following edit:

- By default (unlike posts), page FrontMatter requires a `title` attribute, in order for a link to the page to
- be generated (in `header.html`).
+ Displays a list of Pages with `title` set in the FrontMatter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see where you're coming from, but that doesn't address part of the underlying issue, that no title in Page FrontMatter, leads to... nothing, whereas with a Post, no title DWIM. Do you think your version is explicit enough that it's clear that no Page title => no output?

@ashmaroli
Copy link
Member

Do you think your version is explicit enough that it's clear that no Page title => no output?

Perhaps.. perhaps not.
I'm open for further edits. But, I recommend that it be a short n simple sentence.

The trouble with

By default (unlike posts), page FrontMatter requires a `title` attribute, in order for a link to
the page to be generated (in `header.html`).

is that it doesn't convey the essence in a simple manner. I'll break it out to illustrate.


By default is used for configurable items. For example,

By default the color is red. But you can override it by defining the '$accentColor' variable

(unlke posts) =>

posts or any other collection items have no say here..in the header.html. Its just Pages and Pages alone. Unnecessary confusion.

(in header.html) =>
won't be necessary if the note is included in the right place

@antgel
Copy link
Contributor Author

antgel commented Jul 14, 2017

@ashmaroli Thanks for the feedback, commit amended to suit, let me know what you think.

@ashmaroli
Copy link
Member

LGTM now.

/cc @DirtyF
Should we also make a note about header_links configuration (under Usage) as part of this PR? 🤔

@DirtyF
Copy link
Member

DirtyF commented Jul 15, 2017

Thank you @antgel for the edit! 😄 🔗 💥

@benbalter
Copy link
Contributor

@jekyllbot: merge +docs

Thanks @antgel!

@jekyllbot jekyllbot merged commit c80bf53 into jekyll:master Jul 17, 2017
jekyllbot added a commit that referenced this pull request Jul 17, 2017
@jekyll jekyll locked and limited conversation to collaborators Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants