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

Add first-level headings to navmenu #239

Merged
merged 3 commits into from
Aug 25, 2016
Merged

Add first-level headings to navmenu #239

merged 3 commits into from
Aug 25, 2016

Conversation

mortenpi
Copy link
Member

Add first-level headings to the navigation menu. The only exception is
when the very first block of a page is a first-level heading -- in this
case that particular heading is ignored. This is consistent with how
the headings are usually used, with the first heading being the page
title.

The <li> tags of the first-level headers have the class .toplevel
attached to them, so that we can style them in the CSS.

Fixes #225.


@cormullion and @MichaelHatherly: What I am wondering is whether the behavior that a <h1> is a page title if and only if it is the very first block of a page is reasonable. Also, should we go with this, I guess we should also change the pagetitle function, so that only the very first block would be used to automagically determine a page title for the nav.menu?

We could also maybe use a small doc. note on this.

@cormullion
Copy link
Contributor

In documentation circles, a small amount of selective redundancy is considered acceptable (more so than in coding circles)... So it wouldn't be too bad if the first heading was also used elsewhere. But this solution is also fine -- particularly if added to the really fine manual.

hs = filter(e -> isa(e, Base.Markdown.Header{2}), page.elements)
map(hs) do e
hs = filter(enumerate(page.elements)) do _
i, e = _
Copy link
Member

Choose a reason for hiding this comment

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

Can i and e have more descriptive names?

@MichaelHatherly
Copy link
Member

What I am wondering is whether the behavior that a <h1> is a page title if and only if it is the very first block of a page is reasonable.

Yes, that seems reasonable to me.

@MichaelHatherly MichaelHatherly added this to the 0.3.1 milestone Aug 24, 2016
Add first-level headings to the navigation menu. The only exception is
when the very first block of a page is a first-level heading -- in this
case that particular heading is ignored. This is consistent with how
the headings are usually used, with the first heading being the page
title.

The `<li>` tags of the first-level headers have the class `.toplevel`
attached to them, so that we can style them in the CSS.

Fixes #225.
To be consistent with the sidebar change, where we don't display the
first <h1>, we should also only use the <h1> tag to guess the page
title when it is the very first block of the Markdown file.
@mortenpi
Copy link
Member Author

I think we should treat the page outline consistently, both in the sidebar and when guessing a page title. So I've added a change that we'd only use the very first <h1> block to guess the page title (and also a small doc. note).

Assuming it looks good, I'd propose that we would only backport the first commit -- the second commit changes the behavior of Documenter.

@MichaelHatherly
Copy link
Member

the second commit changes the behavior of Documenter.

It only changes the behaviour of the HTML output, correct? If so then we can get away with changing it in a point release since we've explicitly stated that it is experimental at this point. Probably better to change it sooner rather than later anyway, otherwise people will end up relying on it if we leave it too long.

@MichaelHatherly
Copy link
Member

(Also, LGTM, so merge whenever you're ready @mortenpi.)

@mortenpi mortenpi merged commit 2864ecf into JuliaDocs:master Aug 25, 2016
@mortenpi mortenpi deleted the mp/fix-sidebar branch August 25, 2016 19:28
mortenpi added a commit that referenced this pull request Aug 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants