Skip to content

Fixed broken links, fixed broken anchors, removed duplicate main.css, fixed newline issue with site's description, fixed guide layout, and fixed slug in the guide's sidebar #135

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 6 commits into from
Dec 5, 2017

Conversation

aaronsofaly
Copy link
Contributor

@aaronsofaly aaronsofaly commented May 29, 2016

This PR features lots of bug fixes. Some of these bugs were only present if you browsed your gh-pages version of the site and so they weren't present on chaijs.com. You can see my gh-pages here, http://aaronsofaly.github.io/chai-docs/.

I'll explain each commit.

This commit fixes broken links. Some links didn't have {{site.github.url}}, so when you browsed your gh-pages version of the site then the links would be 404.

This commit fixes broken anchors.

This commit fixes duplicate main.css references in head.html. You can see the duplicate references on lines 10 and 20.

This commit fixes a newline issue in the site's description. I was only seeing this issue on my gh-pages and not on chaijs.com. Here is the original _config.yml. Because the description used a |, the second line of the description was converted to a <br>. I changed it from a | to a > because the > will convert a newline to a space instead of a <br>. This Stackoverflow answers explains it pretty well.

This commit fixes the sidebar in the guide. This one was tricky. The _config.yml file declares that the layout and body classes should be "guide". However, gh-pages is completely ignoring that instead uses the default layout and does not add a body class. Therefore the guide doesn't have a sidebar on the chaijs.com/guide/ site. My local Jekyll compiles the sidebar just fine but for some reason gh-pages doesn't compile it properly and completely ignores what's in _config.yml. So I have moved what was declared in _config.yml to each of the guide pages.

This commit fixes the slugs in the guide's sidebar. The installation guide has a subheading of Node.js. When the subheading is parsed by Markdown then you get <h3 id="nodejs">. However, the guide's sidebar code creates a slug of <a href="#node-js">. So when you click the link in the sidebar it doesn't work because they don't match. So I updated the slug in the sidebar to preserve the period in Node.js and then I use a string filter to remove the period from the slug and so then you get <a href="#nodejs">.

There is still an outstanding issue though. Notice the sidebar on the resources page of the guide. There are two links of Getting Help and Contributing. However, the page content doesn't have these subheadings and so the sidebar links don't work. We need to change the links in the sidebar to match the actual subheadings in the page content.

@astorije astorije self-assigned this Jul 1, 2016
@astorije
Copy link
Member

astorije commented Jul 1, 2016

Hey @aaronsofaly, thanks for this, I'll review as soon as I can!

In the meantime, could you rebase this branch on master to resolve conflicts, please?

@aaronsofaly
Copy link
Contributor Author

Hi @astorije! My repo should now be up to date with this repo.

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

Thanks for this @aaronsofaly! Looks great! Sorry it took so long to get this one merged.

@keithamus keithamus merged commit 89f0377 into chaijs:master Dec 5, 2017
@astorije
Copy link
Member

astorije commented Dec 7, 2017

Oh no, I completely let this slip! Sorry and thank you very much @aaronsofaly and @keithamus!

@aaronsofaly
Copy link
Contributor Author

No worries! Glad to see it merged :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants