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

List all headings and build a hierarchy of the headings inside a markdown file #63

Merged
merged 6 commits into from
Jul 4, 2017
Merged

List all headings and build a hierarchy of the headings inside a markdown file #63

merged 6 commits into from
Jul 4, 2017

Conversation

GeertvanHorrik
Copy link
Contributor

Fixes #59

Still WIP, will also need to update the docs.

@FransBouma
Copy link
Owner

Will wait till it reaches 'out-of-WIP' state ;)

@GeertvanHorrik
Copy link
Contributor Author

Yes, I need the other PR accepted first so I can finish this one.

@FransBouma
Copy link
Owner

Will look into that one now :)

# Conflicts:
#	src/DocNet/INavigationElement.cs
#	src/DocNet/NavigatedPath.cs
#	src/DocNet/NavigationElement.cs
#	src/DocNet/NavigationLevel.cs
#	src/DocNet/SimpleNavigationElement.cs
@GeertvanHorrik
Copy link
Contributor Author

image

@GeertvanHorrik
Copy link
Contributor Author

GeertvanHorrik commented Jul 3, 2017

This PR is ready for review. Once accepted, I'll document it. I've introduced the NavigationContext class so we can easily extend it in the future without causing breaking changes and having to update all signatures.

By default nothing changes (defaults to only heading 2 generation), but you can set the MaxLevelInToC setting in the config. In the example screenshot above, I've set it to 3.

@GeertvanHorrik
Copy link
Contributor Author

image

Some improved CSS usage in the theme.

@GeertvanHorrik
Copy link
Contributor Author

For the ones interested in the CSS (theme.css):

.menu-vertical li.tocentry {
	padding-left: 15px;
	padding-top: 7px;
}

.menu-vertical li.tocentry > ul.currentrelative {
	padding-left: 5px;
	padding-bottom: 7px;
}

.menu-vertical li.tocentry > ul.currentrelative > li.tocentry {
	padding-top: 2px !important;
}

@FransBouma
Copy link
Owner

If I run it on my documentation set, so without the setting, my layout is a bit messed up, as the theme I use isn't updated. But I don't know what to update, as the CSS you copy/pasted above makes the levels more condensed. E.g. if I look at the 2nd level entries in this ToC:
http://www.llblgen.com/Documentation/5.2/LLBLGen%20Pro%20RTF/Using%20the%20generated%20code/gencode_dbspecificfeatures.htm

it's not so condensed, but if I use this same CSS, I won't get this layout but instead get this layout:

2017-07-03_165356

I can of course hack this css back into place, but I wonder what changed that it makes it look like this. (as the CSS hasn't ;))

@FransBouma
Copy link
Owner

FransBouma commented Jul 3, 2017

That previous post is wrong wrt 'condensed'. The shot I posted is what I get when I don't adjust any CSS. So its 2nd level elements are too much adjusted to the right (but the CSS hasn't changed!)

If I apply your CSS to the theme it looks like this:

2017-07-03_165746

(edit) So my question is: what changed in the HTML so the original CSS makes it look out of place and a theme change is needed for that. I understand a theme change is needed for having 3rd level etc. ToC members show up properly ?

@GeertvanHorrik
Copy link
Contributor Author

GeertvanHorrik commented Jul 3, 2017

The only thing I can imagine is that I maybe nest the ul > li different. Basically each nested level (ul list) gets into an <li> list. Maybe that wasn't the case before?

@FransBouma
Copy link
Owner

Will check, I have an older batch here, will compare the page.

@FransBouma
Copy link
Owner

FransBouma commented Jul 3, 2017

You wrap them again in a

<li class="tocentry">
<ul class="currentrelative">

so:

<ul class="currentrelative">
<li class="tocentry"><a href="#sql-server-specific-features">SQL Server Specific features</a></li>
<li class="tocentry"><a href="#oracle-specific-features">Oracle specific features</a></li>
<li class="tocentry"><a href="#firebird-specific-features">Firebird specific features</a></li>
<li class="tocentry"><a href="#db2-specific-features">DB2 specific features</a></li>
<li class="tocentry"><a href="#mysql-specific-features">MySQL specific features</a></li>
</ul>

becomes

<ul class="currentrelative">
<li class="tocentry">
<ul class="currentrelative">
<li class="tocentry"><a href="#sql-server-specific-features">SQL Server Specific features</a></li>
<li class="tocentry"><a href="#oracle-specific-features">Oracle specific features</a></li>
<li class="tocentry"><a href="#firebird-specific-features">Firebird specific features</a></li>
<li class="tocentry"><a href="#db2-specific-features">DB2 specific features</a></li>
<li class="tocentry"><a href="#mysql-specific-features">MySQL specific features</a></li>
</ul>
</li>
</ul>

Not sure if this is avoidable?

@GeertvanHorrik
Copy link
Contributor Author

Let me check where I introduced this issue.

@GeertvanHorrik
Copy link
Contributor Author

Fixed. I was just looping through the headings but we never generate h1 so that can be skipped.

@FransBouma
Copy link
Owner

The CSS is still too tight. Will see if I can correct that.

@GeertvanHorrik
Copy link
Contributor Author

The CSS is still too tight. Will see if I can correct that.

My updated one or the original? I think if you use heading 2 (default value), it looks exactly as before?

@FransBouma
Copy link
Owner

My updated one or the original? I think if you use heading 2 (default value), it looks exactly as before?

Yes, but with 3 it is too tight. The main issue is with wrapping headings where the wrapped part looks like a new heading, but it actually belongs to the line above it ;) When I wrote the original theme I had a lot of problems with this too, how to solve this. Tried a lot of different things, in the end it was as simple as adding a pixel or 2 extra space between headings.

If I use the css:

.menu-vertical li.tocentry {
	padding-left: 20px;
	padding-top: 7px;
}

.menu-vertical li.tocentry > ul.currentrelative {
	padding-left: 7px;
	padding-bottom: 5px;
	margin-top: -3px;
}

.menu-vertical li.tocentry > ul.currentrelative > li.tocentry {
	padding-top: 3px !important;
}

it looks like (4 levels)

2017-07-04_103522

where there's distinction between what's wrapped and what's a heading. With the negative margin-top, I remove the needless extra space between the next level items and its heading, which previously wasn't visible (as there was just 1). I'll add this css to the other themes and merge :)

Good job!

@GeertvanHorrik
Copy link
Contributor Author

Nice, looks really good, thanks for merging! Then I'll fix the other 2 PR's as well (but they are really easy to review).

@FransBouma FransBouma merged commit f6a6421 into FransBouma:master Jul 4, 2017
@FransBouma
Copy link
Owner

merged

@FransBouma
Copy link
Owner

You'll add a couple of lines to the docs? :)

@GeertvanHorrik
Copy link
Contributor Author

You'll add a couple of lines to the docs? :)

As always.

@FransBouma
Copy link
Owner

:)

Switched it on to level 3 on our main docs. http://www.llblgen.com/Documentation/5.2/LLBLGen%20Pro%20RTF/Using%20the%20generated%20code/gencode_dbspecificfeatures.htm I've added one extra rule to the CSS so the first entry in a container of a page is not that close to the heading.

@GeertvanHorrik
Copy link
Contributor Author

I think it looks really nice and much easier to navigate. I have been thinking about making the h2 bold, but that's too intrusive for people using just h2.

@FransBouma
Copy link
Owner

It might be possible to do so, if you emit a h2, h3 etc specific class on the ToC entry I guess. You can then use a css selector which matches if the h2 ToC entry contains a ToC entry somewhere in its children. Bold gets ugly quickly though :) Use F12 browser tools to check quickly but I'd leave it as is indeed.

@GeertvanHorrik
Copy link
Contributor Author

Let's leave as-is for now, we can always introduce these changes in the future (super easy now).

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.

2 participants