-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add nav bar 3/3: add _includes/toc.md #2271
Conversation
This is 3/3 pull requests that, if merged, will add a nav bar to the F' site, like the one shown here: https://chr1st1ansears.github.io/fprime/ (The other 2 PRs are for the fprime-theme repo.) This PR creates a new directory and file within that dir: _includes/toc.md. The file gets referenced in the updated default.html, in the fprime-theme repo. toc.md contains the actual contents of the nav bar. In the future, when y'all need to add/remove items in the nav bar, you can use this file to do so.
Links to other PRs:
|
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.
Couple of preliminary notes as I'm thinking through this
Hey @thomas-bc, I hadn't seen earlier that you resolved the remaining conversation about the bolding, etc. Anyway, I assumed that meant you like the current prototype; I've made a commit to this PR, so the nav bar will match the prototype. Please let me know if there's anything else I can help with! |
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.
@Chr1st1anSears the team and I looked at this and we love it!
A few (last) minor aesthetic adjustments that was suggested:
- could we make the sidebar just a tiny bit wider? Not too much, but just so that it's easier to read
- would it be possible for the sidebar to be full height instead of a box? By that I mean, extend the box to be the full height of the page (or up to the banner with the picture - maybe that would look better) and to seem anchored (i.e. not moving), in the same fashion https://just-the-docs.github.io/just-the-docs/ does it.
If you don't have time to take care of this I'll gladly take care of it, but I figured you may want to take this across the finish line yourself :)
Let me know.
And thanks again for the good work!
PS: did https://chr1st1ansears.github.io/fprime go down between yesterday and today?
@thomas-bc thanks for your feedback; I'm so glad to hear the team likes it! I've added one more commit to the PR for fprime-theme. Please check the prototype site to see if it meets the team's needs (I'm not sure if the site went down, but it's up right now). |
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 good to me. Thanks a lot for the good work @Chr1st1anSears !!
Change Description
This PR creates a new directory and file within that dir: _includes/toc.md. The file gets referenced in the updated default.html, in the fprime-theme repo. toc.md contains the actual contents of the nav bar. In the future, when y'all need to add/remove items in the nav bar, you can use this file to do so.
Rationale
Per issue #2100, this is 3/3 pull requests that, if merged, will add a nav bar to the F' site, like the one shown here: (The other 2 PRs are for the fprime-theme repo.) This might be obvious but if you accept this PR, please merge this one last since doing so will trigger a run of
pages build and deployment
.Testing/Review Recommendations
Since this simply adds a new file and dir, which aren't referenced anywhere else in this repo, I don't think it can really cause any issues. I'd recommend taking a closer look at my PRs to the fprime-theme, repo, and reviewing the affect of those changes, again, by checking out my fork's site here: https://chr1st1ansears.github.io/fprime/
Future Work
I noticed that your pages for the tutorials seem to exist in another repo, and they don't reference fprime-community/fprime-theme/_layouts/default.html. So to get the nav bar to appear on those pages, a reference to that default.html file will need to be added to the tutorials' MD files. Also, this is super specific, but if you visit certain pages on a phone, like the Numerical Types Design page, then the formatting gets messed up, whether you have my nav bar added or not. It seems to have something to do with the tables. So that also might be worth looking into down the road.