[WIP] Allow nested categories with recursive navigation. #982

galli-leo · 2018-06-15T21:02:30Z

Overview

So I started working on allowing nested categories for custom categories. Currently there is no limit on the amount of nesting (i.e. levels). This is achieved through nested partials as well as keeping track of the level, and using that in the header tag (<h{{level}}>).

Current Implementation

I only hacked together the css & templates for fullwidth. The theme changes aren't that drastic and I will try to keep it backwards compatible.
The css needs some rework, not that good at css.
Subsections are added as a separate property on the doc_model object.
Name and overview for a subsection (and subsubsection) is emitted before the tasks. See screenshots

Questions

Would the better option be adding the subsections to the children property instead of it's own? And then doing something like this in mustache?:

<li class="nav-group-name">
  <a class="nav-group-name-link" href="{{path_to_root}}{{url}}"><h{{level}}>{{section}}</h{{level}}></a>
  <ul class="nav-group-tasks">
    {{#children}}
      {{#children.count}}
  <ul class="nav-groups">
    {{#children}}
      {{> nav_section}}
    {{/children}}
  </ul>
      {{/children.count}}
      {{^children.count}}
    <li class="nav-group-task">
      <a class="nav-group-task-link" href="{{path_to_root}}{{url}}">{{name}}</a>
    </li>
    {{/children.count}}
  </ul>
</li>

This however would then create complications with tasks, or would it?
Should the full subsection be rendered out before the tasks come? IMO this would get quite lengthy with a lot of sub and subsubsections.
Should the default theme autohide subsubsections (or even subsections)? i.e. make it show on clicking the parent section?
Should there be no indentation in the navigation menu?
Should I add some tests?

Generally, I would really appreciate some help with the css and generally the look & feel.

Screenshots

Created with the following yaml:

module: TestFramework
abstract: Documentation/*.md
documentation: Guide.md
output: docs
theme: fullwidth
custom_categories:
  - name: Main
    children:
      - Test
      - name: Sub
        children:
          - WithEnum
          - name: SubSub
            children:
              - Guide

Linked

Closes #624

johnfairh · 2018-06-16T10:17:51Z

Thanks for the PR, brave place to start! Will look at the code a bit more later.

I think the yaml format is OK; agree that just handling whatever depth the user wants is probably fine.

On the design, I'm not sure users will expect the child abstract hierarchy to appear in each parent abstract. It feels to me as though it distracts from the discussion of the parent topic. Maybe include the child subsections instead in a special Task at the bottom of the page task list that actually links in the normal way (with twisty) to the child section.

On the left nav, I guess I think yes the subsection structure should be reflected with some kind of indentation - wish I could help more on the style front!

You should use the misc_jazzy_features specs project to include tests for the feature.

galli-leo · 2018-06-16T11:39:40Z

@johnfairh That‘s the integration tests right?

Yeah I am not too keen on each abstract as well. What‘s twisty? And yeah, the headers should probably link, didn‘t have any time yesterday to integrate that (though it should be easy).

1ec5 · 2018-06-16T16:31:27Z

What‘s twisty?

Pretty sure it’s a disclosure triangle, like this:

johnfairh · 2018-06-17T10:01:27Z

(Sorry and ty @1ec5 for translating!)
Right, so basically include the subsection links as normal children in the body doc with a task heading (== the heading used for // MARK comments).

galli-leo · 2018-06-17T11:53:27Z

@johnfairh Ah gotcha. Question would be, whether to keep the subsections seperate or add them to the tasks array.

johnfairh · 2018-06-19T13:54:17Z

New thought: we should just include the subsection link in the children array where the user wrote it in their markdown, I think that's the least surprising behavior. [Letting markdown specify headings is a separate issue!]

galli-leo · 2018-06-19T21:58:34Z

@johnfairh I don't quite follow what you mean. Can you elaborate?

galli-leo · 2018-06-28T07:10:05Z

@johnfairh Did you have some time to look at the implementation and decide whether to use a new property (like the current implementation) or use the existing children array? When that‘s decided, I can start working on tests and fixing up things.

Sorry if I am stressing you 😬, this would help a ton with our docs :)

johnfairh · 2018-06-28T17:13:18Z

Sorry missed I owed you a response here! Yes I looked through it, I think you've nicely captured the recursive building part of it and that it will work to treat nested categories as regular children.

PR #985 is a step on the way, deals with linking from category pages to items without an abstract; I'm planning to look at the left nav design/layout part next.

galli-leo · 2018-06-28T20:28:23Z

@johnfairh Cool, will try to get it changed to use children and tasks over the next few days.

JUSTINMKAUFMAN · 2018-06-29T15:59:28Z

As mentioned elsewhere, I think this is a great idea. I’d love to see project directory parsing as a custom category source included in this PR. Wondering if I can help in any way?

johnfairh · 2018-06-30T13:16:34Z

@galli-leo dw, I got it - take a look at this branch & see what you think. I've treated the nested categories like everything else in the left nav so they are less heavy than in yours above, 🤷‍♂️ if this is OK - mostly been persuading the styles to nest reasonably.

@JUSTINMKAUFMAN would be a separate PR. You would need to write a version of this function that builds the category structure from the types returned by SourceKitten - they each have a file attribute with the pathname.

galli-leo · 2018-06-30T14:10:12Z

@johnfairh Well also just got done, treating them like normal children 😬 . Also your mustache templates will never render, since you will have infinite recursion. If there is no children in the current context (i.e. a non category) mustache will try the parent context. Obviously that one has children, so it will render the parent nav again and again. See my branch for how I fixed that.

I like the idea of creating a new type, that makes a lot of sense.

I still think we should include the level, so that we can always use the correct h tag. I agree that they are quite heavy in my implementation, but that's mostly because of the style sheets. I think gradually lowering e.g. font-size for sub and subsubsections makes a lot of sense, while still letting them standout to the rest.

What do you think of my tasks implementation? When you click on a subcategories heading, it discloses that one's abstract now. We could also include the subsections in the normal tasks array, though I don't know how that will play out with mark comments. What do you think? Also would it make sense to include the child items of the category in the "popup"?

Live demo of the latest commit is here: https://galli.me/jazzy-demo/nested

johnfairh · 2018-06-30T15:15:04Z

The templates are fine lol - easy to misread 😉

Yep including the abstract if there is one (suspect often will not be) makes sense - should come out easily as part of treating the nested category like other children [I think this is what you mean by "including in the normal tasks array"]

The design you can see in eg. classes is to include just the abstract in the parent page and leave the next level of children to the next page down.

I misunderstood how much help you were asking for with the visual design here - will let you get on!

galli-leo · 2018-06-30T16:51:40Z

@johnfairh Should I merge in your branch? Or are you planning on implementing this?

johnfairh · 2018-06-30T19:59:15Z

Take as much/little as you need - if you need to do things a different way to get the visual effects you want for the headings and on the category pages then go ahead..

galli-leo · 2018-07-07T10:30:35Z

@johnfairh So I integrated some of your changes (namely moving categories into their own class) into my pr. I think it's mostly ready for a review now, works quite well imo. Checkout https://galli.me/jazzy-demo/nested for a visual demo. Or should I add some tests (and fix the old ones due to adding headings in the nav) before a review?

I made it, so that the Sub categories follow the same style as a mark heading and reveal the abstract as well as their items on clicking on it. Let me know, if I should also include the sub sub categories in that reveal. Should I try to fix the js code not revealing the sub category as well as an item of the subcategory when clicked on?

Furthermore, should I try to fix those cyclomatic complexity errors from rubycop, or just ignore them with a comment? Not sure why it complains about the spacing for self.level = level as well, seems to be the same as your self.name = name.

…ed an example with nested categories.

johnfairh

Thanks!

Added some code nitpicks & a less trivial thought about category names.

I'm pretty sure that the left nav style needs to remain the same as now, certainly for sites that aren't using nested categories -- right now (no nested categories) it goes from:
to

From a design point of view, the sidebar is almost always less important than the body so needs not to have these big highly-spaced headings that are more eye-catching than stuff in the body.

The category headings in the body should probably use a different class or something from the mark headings: we know that some users do not like the mark headings and use custom CSS to suppress them.

There are various existing problems with the js reveal/hide code -- I would save fixes for another PR.

Yes, strong default action is to fix the rubocop errors - they're usually pointing out good Ruby style points.

Either need apple + jony theme changes or docs + something to detect the incompatibility -- generated html looks OK at first but behaves pretty oddly.

I haven't checked how this looks in Dash.

johnfairh · 2018-07-09T10:13:37Z

lib/jazzy/doc_builder.rb

@@ -105,6 +111,13 @@ def self.each_doc(output_dir, docs, &block)
          doc.children,
          &block
        )
+        if doc.subsections != nil


Change not required, subsections unused.

johnfairh · 2018-07-09T10:14:22Z

lib/jazzy/doc_builder.rb

+        alternative_abstract = subsection.alternative_abstract
+        if alternative_abstract
+          overview = render(subsection, alternative_abstract) + overview
+        end


Better to pull out these 5-6 lines instead of duplicating them.

johnfairh · 2018-07-09T10:17:47Z

lib/jazzy/source_category.rb

+
+    def initialize(group, name, abstract, url_name, level = 1)
+      super()
+      self.type     = SourceDeclaration::Type.category


I'm not sure if this is supposed to be rename of Type.overview or something else - if a rename then need to delete overview and add a category? helper to get rid of the manual comparisons everywhere. Usually better to do refactorings in separate commits to new code, makes it easier for people to read.

Not a rename, an additional type. I think it makes the filter and partition calls in doc_builder more logical. Will add a helper.

Should I move this to a separate commit? Or just leave it for now?

Ok. overview is the type used for these category pages. You could rename it (as long as this doesn't change existing URLs) but we don't need an additional type.

johnfairh · 2018-07-09T10:18:14Z

lib/jazzy/source_category.rb

+      self.abstract = Markdown.render(abstract)
+      self.children = group
+      self.parameters = []
+      self.mark       = SourceMark.new


This doesn't need a mark if it's not going into the children/tasks render structure.

Yeah, I left it from your code. Will remove.

johnfairh · 2018-07-09T10:19:37Z

lib/jazzy/source_category.rb

+      self.children = group
+      self.parameters = []
+      self.mark       = SourceMark.new
+      self.level    = level


Rubocop complains about this line because it can't figure out what the spaces are for: either use just 1 or line up the = with the line above.

A gotcha, will fix.

johnfairh · 2018-07-09T10:20:06Z

lib/jazzy/source_declaration.rb

@@ -99,6 +99,8 @@ def display_other_language_declaration
    attr_accessor :end_line
    attr_accessor :nav_order
    attr_accessor :url_name
+    attr_accessor :level
+    attr_accessor :subsections


(subsections not used any more)

johnfairh · 2018-07-09T10:40:46Z

lib/jazzy/sourcekitten.rb

-          doc.children = make_doc_urls(doc.children)
+          if doc.children.count > 0
+            doc.children = make_doc_urls(doc.children)
+          end


I think undo this change, original code is better.

Might be leftover from your branch with the other changes, will remove.

johnfairh · 2018-07-09T11:13:29Z

lib/jazzy/sourcekitten.rb

-          doc.children = make_doc_urls(doc.children)
+          if doc.children.count > 0
+            doc.children = make_doc_urls(doc.children)
+          end


This url construction part leaves all category pages in the same directory. I'm not sure it's obvious that users should not duplicate subcategory names even if they are in different parent trees.

Would you rather have them moved to subdirectories or have a note in the config info?

I thought about adding a section to the README anyways.

Requiring subcategory name uniqueness feels like a footgun/defect creator. Putting the pages in subdirs should work.

johnfairh · 2018-07-09T11:14:57Z

lib/jazzy/themes/fullwidth/templates/doc.mustache

@@ -49,7 +49,7 @@
        </section>

        {{> tasks}}
-
+        


(can tidy this up)

galli-leo · 2018-07-09T21:14:05Z

@johnfairh

Added some code nitpicks & a less trivial thought about category names.

Should have time tomorrow to fix those.

I'm pretty sure that the left nav style needs to remain the same as now, certainly for sites that aren't using nested categories -- right now (no nested categories) it goes from:
screen shot 2018-07-09 at 12 31 10 to screen shot 2018-07-09 at 12 32 32

From a design point of view, the sidebar is almost always less important than the body so needs not to have these big highly-spaced headings that are more eye-catching than stuff in the body.

Do you propose going back to just tags or styling the headings differently? I agree it's a bit too big, I just wanted to make the distinction between subsections clear for demo purposes.

The category headings in the body should probably use a different class or something from the mark headings: we know that some users do not like the mark headings and use custom CSS to suppress them.

Will add a custom CSS class. Should the style be still the same?

Yes, strong default action is to fix the rubocop errors - they're usually pointing out good Ruby style points.

Should have time to fix those tomorrow as well.

Either need apple + jony theme changes or docs + something to detect the incompatibility -- generated html looks OK at first but behaves pretty oddly.

Yeah I wanted to have one theme finalized, before working on the other ones. Will do those when fullwidth is good to go.

I haven't checked how this looks in Dash.

I have taken a quick look and it seems fine, but the sections are unfortunately not nested inside dash. Not sure if that's possible or not.

johnfairh · 2018-07-10T10:59:53Z

[sidebar] just tags or styling the headings differently?

I guess find some other way of styling the 'sub'categories (same weight as the top categories?) - or leave them styled as they come out and rely on the indentation to indicate the hierarchy.

[body category headings] Should the style be still the same?

Yes I think that makes sense.

wanted to have one theme finalized, before working on the other ones. Will do those when fullwidth is good to go.

Sounds good!

…ied up css and fixed a few other things.

galli-leo · 2018-07-12T20:51:05Z

lib/jazzy/source_category.rb

+        children = category['children'].flat_map do |child|
+          if child.is_a?(Hash)
+            # Nested category, recurse
+            children, docs = group_custom_categories(docs, [child], level + 1)


@johnfairh I renamed docs_with_name to children to a) make rubocop happy and b) make it's purpose clearer.

Is that ok?

galli-leo · 2018-07-12T20:55:47Z

@johnfairh I toned the navigation stylings down a bit. The only change now is font-weight, though I had to increase the font-size a little, or else the headings looked smaller (not sure why).

Before:

After:

Can tone them down even more, if you think that's better. IMO having some differentiation through font-weight in addition makes it a bit easier to read.

johnfairh · 2018-07-14T11:20:10Z

Code changes look great. I think the level property is redundant now, can get it dynamically from the length of documentation_path.

I appreciate the work on the styling. There's still additional vertical whitespace above heading elements (?); I would like the default weights to be as they are currently -- you can always override in your own projects.

galli-leo · 2018-07-14T13:07:49Z

@johnfairh Sounds good. Moved level to a computed property (not sure if that's what you call it in ruby). Also made the categories look like before. I still added some top margin to all sub and subsub categories, because they would have had less spacing to the items before them than after them.

johnfairh · 2018-07-23T18:04:00Z

Thanks! This all looks good to me now -- code nits that you can get rid of the attr_accessor :level line, and the changelog is missing the double-space newline.

If you could update your demo site to reflect the most recent theme update + @jpsim can OK the design then I reckon good to update the other themes and merge it.

(sorry for the delay: this time my excuse is that GitHub doesn't email comment edits!)

galli-leo · 2018-07-23T21:21:24Z

Fixed both and updated the demo site as requested.

BTW. maybe it would be a cool idea to host the generated integration specs somewhere? So a reviewer and even pr author can easily look at changes? Additionally, a visual diff would be even cooler. Is that something you have considered?

(sorry for the delay: this time my excuse is that GitHub doesn't email comment edits!)

No worries! Out of curiosity, do you get notifications for commits to a pr? If not should I leave messages for new commits in the future?

johnfairh · 2018-07-25T16:28:29Z

👍 thanks.

FYI the specs are in the spec/integration_specs directory -- if you do bundle exec rake rebuild_integration_specs then they will build with the current code and you can git diff to verify page-source level changes, and do a linked PR for that project that is easy for people to compare/clone + do whatever visual diff (as well as make the jazzy tests all be green). Yes there's probably some opportunity to do some scripting or something to make this easier.

Yep, commits notify! It might just be me but I usually wait for a comment to indicate "OK i've stopped pushing stuff, please take a look" :-)

galli-leo · 2018-07-29T14:16:49Z

@johnfairh I will wait on the integration_specs pr until all themes are finished.

@jpsim If you are ok with the design, I can start on the other themes :)

galli-leo · 2018-08-07T20:00:16Z

Bump

nrudnyk · 2019-10-05T17:29:56Z

@galli-leo @johnfairh is there any progress/plans with this feature?

manand-ipratico · 2021-01-28T14:10:20Z

Is there any update for this feature?
thanks

Initial addition of nested categories

55d469f

realm-ci assigned galli-leo Jun 15, 2018

Moved subsections back to the children property.

0c899ac

Moved categories to their own class and fixed up the css.

14b567b

Updated config file to include hint about nested categories. Also add…

c9c597e

…ed an example with nested categories.

johnfairh requested changes Jul 9, 2018

View reviewed changes

Fixed rubocop errors, made subcategory urls go to subdirectories, tid…

d8b2f27

…ied up css and fixed a few other things.

galli-leo commented Jul 12, 2018

View reviewed changes

Forgot one instance of Type.overview.

baf5989

galli-leo added 5 commits July 14, 2018 15:14

Moved level to a computed property.

ab9b537

Changed styles to be like the original categories.

6cbe723

Updated Readme to include info about custom categories.

7c400eb

Updated Changelog

e263604

Make rubocop happy again.

4c6c76c

galli-leo added 2 commits July 23, 2018 23:12

Remove unecessary level accessor

bee9a9d

Fixed Changelog

75e4721

johnfairh mentioned this pull request Nov 17, 2019

support nested custom_categories #624

Open

nolan-yoo-miris mentioned this pull request Sep 9, 2023

Custom categories from code comments #1170

Open

[WIP] Allow nested categories with recursive navigation. #982

Are you sure you want to change the base?

[WIP] Allow nested categories with recursive navigation. #982

Conversation

galli-leo commented Jun 15, 2018

Overview

Current Implementation

Questions

Screenshots

Linked

johnfairh commented Jun 16, 2018

galli-leo commented Jun 16, 2018 • edited Loading

1ec5 commented Jun 16, 2018

johnfairh commented Jun 17, 2018

galli-leo commented Jun 17, 2018

johnfairh commented Jun 19, 2018

galli-leo commented Jun 19, 2018

galli-leo commented Jun 28, 2018

johnfairh commented Jun 28, 2018

galli-leo commented Jun 28, 2018

JUSTINMKAUFMAN commented Jun 29, 2018

johnfairh commented Jun 30, 2018

galli-leo commented Jun 30, 2018 • edited Loading

johnfairh commented Jun 30, 2018

galli-leo commented Jun 30, 2018

johnfairh commented Jun 30, 2018

galli-leo commented Jul 7, 2018 • edited Loading

johnfairh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnfairh Jul 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galli-leo commented Jul 9, 2018

johnfairh commented Jul 10, 2018

galli-leo Jul 12, 2018 • edited Loading

Choose a reason for hiding this comment

galli-leo commented Jul 12, 2018

johnfairh commented Jul 14, 2018

galli-leo commented Jul 14, 2018 • edited Loading

johnfairh commented Jul 23, 2018

galli-leo commented Jul 23, 2018

johnfairh commented Jul 25, 2018

galli-leo commented Jul 29, 2018

galli-leo commented Aug 7, 2018

nrudnyk commented Oct 5, 2019

manand-ipratico commented Jan 28, 2021 • edited Loading

galli-leo commented Jun 16, 2018 •

edited

Loading

galli-leo commented Jun 30, 2018 •

edited

Loading

galli-leo commented Jul 7, 2018 •

edited

Loading

johnfairh Jul 10, 2018 •

edited

Loading

galli-leo Jul 12, 2018 •

edited

Loading

galli-leo commented Jul 14, 2018 •

edited

Loading

manand-ipratico commented Jan 28, 2021 •

edited

Loading