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 an option to hide pages in sidebar #282

Merged
merged 5 commits into from
Oct 17, 2016
Merged

Add an option to hide pages in sidebar #282

merged 5 commits into from
Oct 17, 2016

Conversation

mortenpi
Copy link
Member

Adds the Hidden type that can be used in the pages option of makedocs to hide but still build a list of pages in the sidebar.


So, I'd RFC this implementation. Sorry that it took me a while to get to this. Ref #230.

@tlnagy: I also built the Gadfly docs (build here) when using the Hidden feature (make.jl here). Feedback would be greatly appreciated.

I went with the Hidden(page, [hidden_pages]) option in the pages variable. Regarding how things get displayed:

  • Prev/Next links currently work as if the hidden pages were normal subpages (i.e. the page will point to the first of hidden_pages etc.; example ). What @tlnagy suggested might be better (to have the visible pages ignore the hidden ones in the next/prev).
  • The hidden page does not show up in the sidebar at all, even if its the page being looked at, and the parent page gets highlighted in the menu. It does show up in the header though (e.g. » Manual » Guides » Guide.xlabel). Maybe we should have e.g. "Guide » Guide.xlabel" in the sidebar, but this could be too long sometimes.
  • The heading listing always corresponds to the page currently being looked at, but the parent page and hidden page listings are not distinguished at all. I feel that this, when the sidebar doesn't make it obvious that we're on one the hidden pages, might be slightly confusing.

WIP since the tests need work.

@MichaelHatherly
Copy link
Member

Sorry that it took me a while to get to this.

No worries, glad to see you've still got some spare time to work on it!

Prev/Next links currently work as if the hidden pages were normal subpages (i.e. the page will point to the first of hidden_pages etc.; example ). What @tlnagy suggested might be better (to have the visible pages ignore the hidden ones in the next/prev).

I'm partial to keeping the hidden pages in the prev/next links. I quite like how the "flow" is still retained through the docs, instead of skipping whole chunks of it — we don't want hidden pages to be truely hidden, just not cluttering up the nav menu (at least that's been my interpretation of the issue so far).

Maybe we should have e.g. "Guide » Guide.xlabel" in the sidebar, but this could be too long sometimes.

Would definitely get too long at times, I think just listing it at the top would be sufficient.

I feel that this, when the sidebar doesn't make it obvious that we're on one the hidden pages, might be slightly confusing.

Agreed, maybe we could have the navigation text (breadcrumbs, next/prev, and current nav link) in italics instead to make it more obvious that the page is different.


Syntax-wise I'd be inclined to have this work per-page rather than per-section to make it more "composable", ie.

# ...
"Section One" => [
    "page.md",
    "Hidden" => hide("next-page.md"),
    "Another Page" => "other-page.md",
]

and just map it if you've got an entire section that you want hidden:

# ...
"Secret Section" => map(hide, [
    "One" => "one.md",
    "two.md",
]

Would it be possible to implement it this way?

@MichaelHatherly
Copy link
Member

With that definition of hide the current Hidden would just be a 2-arg method of hide defined as

hide(page::AbstractString, others::Vector) = vcat(page, map(hide, others))

presumably.

@tlnagy
Copy link
Contributor

tlnagy commented Sep 22, 2016

This looks great @mortenpi! Thanks for starting to work on this. A couple things

I'm partial to keeping the hidden pages in the prev/next links. I quite like how the "flow" is still retained through the docs, instead of skipping whole chunks of it — we don't want hidden pages to be truely hidden, just not cluttering up the nav menu (at least that's been my interpretation of the issue so far).

I'm fine with either solution. I don't really use the next/prev ¯_(ツ)_/¯

Syntax-wise I'd be inclined to have this work per-page rather than per-section to make it more "composable", ie.

I think the map solution would work well.

The hidden page does not show up in the sidebar at all, even if its the page being looked at, and the parent page gets highlighted in the menu. It does show up in the header though (e.g. » Manual » Guides » Guide.xlabel). Maybe we should have e.g. "Guide » Guide.xlabel" in the sidebar, but this could be too long sometimes.

I would prefer if the hidden page shows up in the sidebar. I think it is confusing that arguments and and examples show up under Geometries instead of under Guide.ylabel here: http://mortenpi.eu/Gadfly.jl/lib/guides/guide_ylabel.html#Guide.ylabel-1, for example.

Regardless, huge improvement 🎉

@MichaelHatherly
Copy link
Member

MichaelHatherly commented Sep 22, 2016

Another option instead of the hide/Hidden approach would be to just use a naming convention for hidden files. Something like a leading underscore could work: lib/_guide_label.md, which has the nice side-effect of being visible when just browsing the directory, rather than needing to refer to the make.jl file. (Just a thought though, not particularly attached to that idea.)

@tlnagy
Copy link
Contributor

tlnagy commented Sep 22, 2016

What about the unix convention of hidden files? .somefile.md?

@MichaelHatherly
Copy link
Member

Was also thinking of that one, but would be a bit annoying perhaps to have them actually hidden when using ls etc. on the directory.

@mortenpi
Copy link
Member Author

Thanks for the input!

  • I also like the flow feeling, so I think we can leave the prev/next as is then.

  • I think that's a good thought to have the hidden page (and just the current one) just show up if it happens to be the current page. A quick mock-up:
    hidden_pages

  • I don't mind making this more versatile, but do we actually want to allow the hiding of just some of the pages of a particular section? Having just a few of the pages sort of randomly be hidden might be a bit weird/confusing and I don't see the use case at the moment.

    Implementation-wise should be fine though.. I should just treat the hiddenness to be a property of a page, not a group of pages. Might, in fact, simplify code.

    I do think we need the two argument hide though, for the Gadfly use case, where you want to have a
    page with an index etc. We currently can't specify a page to be rendered for the non-leaf nodes.

  • +1 to calling it hide.

  • I am currently slightly leaning towards having the sidebar be completely defined in make.jl and not rely on cues from the file names. However, if we go with it, I would go with the underscore, so that the files wouldn't get hidden from the developer.

@MichaelHatherly
Copy link
Member

I think that's a good thought to have the hidden page (and just the current one) just show up if it happens to be the current page. A quick mock-up:

👍

Having just a few of the pages sort of randomly be hidden might be a bit weird/confusing and I don't see the use case at the moment.

Could be, though I'd say it's just up the authors to do what fits with there docs. I can't, at the moment, think of a particular use case, but it just seems a bit off to restrict it to sections only.

I should just treat the hiddenness to be a property of a page, not a group of pages.

👍

I do think we need the two argument hide though

Fine by me. Having a couple of convenience defs for common cases seems like a good idea to me.

I am currently slightly leaning towards having the sidebar be completely defined in make.jl and not rely on cues from the file names.

That's fine by me then. (Since nav structure also isn't dictated by folder structure either.)

@MichaelHatherly
Copy link
Member

Also just wanted to add that having Gadfly's docs available to test things out on is great, thanks @tlnagy!

@mortenpi
Copy link
Member Author

Having a couple of convenience defs for common cases

It wouldn't be a convenience def though, unless I am missing something, since you can't do

 - A => "A.md"
   | - "x" => "A/x.md" (hidden)
   | - "y" => "A/y.md" (hidden)

without it right now (i.e. to have "A" point to a page).

I was briefly thinking that maybe we should allow that somehow, even for cases where we don't hide the pages.

"A" => "A.md" => [
    "x" => "A/x.md",
    "y" => "A/y.md"
]

and

"A" => "A.md", [
    "x" => "A/x.md",
    "y" => "A/y.md"
]

crossed my mind, but I am not a particular fan of either.

Since nav structure also isn't dictated by folder structure either.

Yup, this came up before. I think that my general feeling about it is that, in principle, using the file system would be great, but we should go all in then. Since it seems that we need things that we cannot (conveniently) infer from the file system, and hence need to have the pages anyway, it would be better not to mix approaches. (But I am not sure if it really applies here.)

@MichaelHatherly
Copy link
Member

It wouldn't be a convenience def though, unless I am missing something, since you can't do

Perhaps not then. Hadn't actually tried to see whether it would work.

but I am not a particular fan of either.

Agreed.

it would be better not to mix approaches

👍

@tlnagy
Copy link
Contributor

tlnagy commented Sep 22, 2016

Also just wanted to add that having Gadfly's docs available to test things out on is great, thanks @tlnagy!

Thanks to you all too for accommodating my weird requests :p

A quick mock-up

Looks great!

it would be better not to mix approaches

👍

@mortenpi mortenpi mentioned this pull request Sep 28, 2016
3 tasks
@tlnagy
Copy link
Contributor

tlnagy commented Oct 13, 2016

Any progress on this? This would be nice!

@mortenpi
Copy link
Member Author

Any progress on this? This would be nice!

I literally started working on it again four minutes ago 😆

@tlnagy
Copy link
Contributor

tlnagy commented Oct 13, 2016

I read your mind haha 😂

@mortenpi
Copy link
Member Author

Switched to hide. I still need to implement the showing of the page title if the hidden page is open and do some cosmetic and doc updates. I hope to continue tomorrow evening.

@mortenpi mortenpi changed the title WIP: Add an option to hide pages in sidebar Add an option to hide pages in sidebar Oct 14, 2016
@mortenpi
Copy link
Member Author

Ready for another set of eyes I think. Example build with Gadfly docs (make.jl) updated too. Couple of changes not directly related to this PR as well that cropped up as I was working on this.

@MichaelHatherly
Copy link
Member

All looks good to me, thanks! The hidden pages in the Gadfly docs definitely make the site look much smaller than it actually is, but I think that's probably fine. Let's let @tlnagy have a look over as well though if he's got some time.

(This is probably a big enough change to warrant a new release-0.6 rather than backporting.)

@tlnagy
Copy link
Contributor

tlnagy commented Oct 15, 2016

That looks great! 👍

the hidden pages in the Gadfly docs definitely make the site look much smaller than it actually is

I think it makes the site a lot less overwhelming, which is good.

Also, @mortenpi if you want to submit a PR to Gadfly eventually with these changes, I would be happy to merge them

In case something goes wrong with LaTeX compilation, it stops and asks
for user input. That is not ideal for automated builds and tests.
Passing -interaction=nonstopmode to the latexmk command should stop it
from doing that and just make the command fail in that case.
Throwing an error if the repo slugs do not match means that Travis
builds of forks will always fail. This should instead be a warning
instead to inform the user. should_deploy already makes sure that the
deployment does not happen on forks.
Currently collect_subsections will ignore all the initial level 1
headers (i.e. if there are several without level 2 headers in-between),
but it should ignore only the very first one. Style change is necessary
so that the border between page title and first in-page heading would
not get too thick.
Adds the `hide` function to the public API that can be used in the
`pages` option of `makedocs` build a list of pages that will not be
shown in the sidebar.

Fixes #230.
@mortenpi
Copy link
Member Author

One final thing: I propose hiding the pages under Library > Internals with this. Demo here: http://mortenpi.eu/Documenter.jl/hide/

@MichaelHatherly
Copy link
Member

Yup, fine by me.

@mortenpi mortenpi merged commit b7c4d5d into JuliaDocs:master Oct 17, 2016
@mortenpi mortenpi deleted the hidden-pages branch October 17, 2016 08:11
@MichaelHatherly
Copy link
Member

Might as well tag 0.6 now, rather than 0.5.2 which those backports were for. Feel free to do the branching and tagging, otherwise I'll get around to it later today.

@tlnagy
Copy link
Contributor

tlnagy commented Oct 31, 2016

Hey @mortenpi, whenever you get a chance could you submit your changes as a PR. It would be great to have the cleaned up docs for Gadfly.

@mortenpi
Copy link
Member Author

GiovineItalia/Gadfly.jl#923? 😆 It does seem that the build on the nightly for that commit failed though (due to a buggy nightly, I believe, not due to Documenter), so the docs were not updated. Re-running that job should deploy the new docs I think.

@tlnagy
Copy link
Contributor

tlnagy commented Oct 31, 2016

Ah yes. Whoops. I just saw that the pages were still not hidden. I'll try and force rebuild. thanks!

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