8000 Fix titles redux by lurch · Pull Request #1971 · raspberrypi/documentation · GitHub
[go: up one dir, main page]

Skip to content

Fix titles redux #1971

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 1 commit into from
Aug 9, 2021
Merged

Fix titles redux #1971

merged 1 commit into from
Aug 9, 2021

Conversation

lurch
Copy link
Contributor
@lurch lurch commented Aug 9, 2021

No description provided.

@lurch lurch requested a review from mudge August 9, 2021 11:11
@lurch lurch added the toolchain This is an infrastructure/toolchain issue label Aug 9, 2021
@aallan
Copy link
Contributor
aallan commented Aug 9, 2021

@lurch Is this fixable?

Screenshot 2021-08-09 at 12 14 39

The title of the documentation/ is "Raspberry Pi Documentation - index" any way to make that "Raspberry Pi Documentation" without breaking the rest of the titles.

@lurch
Copy link
Contributor Author
lurch commented Aug 9, 2021

Not easily... 😕

@lurch
Copy link
Contributor Author
lurch commented Aug 9, 2021

Alternatively we could make it say "Raspberry Pi Documentation - Computers" since the "index page" is a copy of the "computers category page"? 🤷

@aallan
Copy link
Contributor
aallan commented Aug 9, 2021

Alternatively we could make it say "Raspberry Pi Documentation - Computers" since the "index page" is a copy of the "computers category page"? 🤷

That's bad for SEO. 😢

Any way to just hardwire it?

@lurch
Copy link
Contributor Author
lurch commented Aug 9, 2021

Okay, let me try a different tactic...

@lurch
Copy link
Contributor Author
lurch commented Aug 9, 2021

...hmmm, nope my alternative tactic:

diff --git a/jekyll-assets/_layouts/docs.html b/jekyll-assets/_layouts/docs.html
index 0e187ffb..346978c8 100644
--- a/jekyll-assets/_layouts/docs.html
+++ b/jekyll-assets/_layouts/docs.html
@@ -14,7 +14,7 @@
       {% include nav.html %}
 
       <section id="content">
-        <h1>{{ page.title }}</h1>
+        <h1>{{ page.sub_title }}</h1>
         {{ content }}
       </section>
     </div>
diff --git a/scripts/create_build_adoc.py b/scripts/create_build_adoc.py
index c6796ab7..c2577e7b 100755
--- a/scripts/create_build_adoc.py
+++ b/scripts/create_build_adoc.py
@@ -30,7 +30,8 @@ if __name__ == "__main__":
         out_fh.write(""":parentdir: {}
 :page-layout: docs
 :includedir: {}
-:doctitle: Raspberry Pi Documentation - {}
+:doctitle: {}
+:sub_title: {}
 
 include::{{includedir}}/{{parentdir}}/{}[]
-""".format(output_path, src_dir, index_title, adoc_filename))
+""".format(output_path, src_dir, 'Raspberry Pi Documentation - {}'.format(index_title), index_title, adoc_filename))

didn't work as it ends up with the <h1> tags in the generated docs pages (e.g. documentation/computers/linux_kernel.html) being empty 🙁 (Looks like the Jekyll template can't "see" the sub_title variable? 🤷 )
I think this might be one for @nelliemckesson as it goes beyond my basic Jekyll knowledge!

@aallan
Copy link
Contributor
aallan commented Aug 9, 2021

Okay. I'm going to merge your intial fix then (aka this PR) that fixes everything except the top level index header.

Thanks!

@aallan aallan self-requested a review August 9, 2021 12:05
Copy link
Contributor
@aallan aallan left a comment

Choose a reason for hiding this comment

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

Need to fix the top level title so it's "Raspberry Pi Documentation" rather than "Raspberry Pi Documentation - Index" but other than that, LGTM.

@aallan aallan merged commit 5833f33 into develop Aug 9, 2021
@aallan aallan deleted the fix_titles_again branch August 9, 2021 12:07
@aallan aallan added the bug fix label Aug 9, 2021
@lurch
Copy link
Contributor Author
lurch commented Aug 9, 2021

(Looks like the Jekyll template can't "see" the sub_title variable? 🤷 )

I took a lucky guess at the fix and it worked 🎉 #1980

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix toolchain This is an infrastructure/toolchain issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0