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

Remove assumption that the table of contents will be an <ol>. #1

Merged
merged 1 commit into from
Sep 23, 2013

Conversation

Osmose
Copy link
Contributor

@Osmose Osmose commented Sep 20, 2013

A recent change to anolis[1] caused it to output the table of contents
using <ul> tags instead of <ol> tags, breaking the generation of the
table of contents for individual pages.

Instead, we now search for the table of contents by the toc class
without assuming what tag it will use.

[1] https://bitbucket.org/ms2ger/anolis/commits/62d9a108439d70e33ad2bb3b4009efd3dbf9cac5

for c in li.iterchildren():
if c.tag == 'a':
if c.get('href')[0] == '#':
items.append( (depth, c.get('href')[1:], c) )
elif c.tag == 'ol':
elif c.attrib['class'] == 'toc':
Copy link
Member

Choose a reason for hiding this comment

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

This instead assumes that nested lists will have the 'toc' class, which is configurable in anolis. Check for "ol" or "ul" instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

A recent change to anolis[1] caused it to output the table of contents
using <ul> tags instead of <ol> tags, breaking the generation of the
table of contents for individual pages.

Instead, we search for <ul> or <ol> tags.

[1] https://bitbucket.org/ms2ger/anolis/commits/62d9a108439d70e33ad2bb3b4009efd3dbf9cac5
darobin added a commit that referenced this pull request Sep 23, 2013
Remove assumption that the table of contents will be an <ol>.
@darobin darobin merged commit 9237b3e into w3c:master Sep 23, 2013
@Osmose Osmose deleted the fix-ol-assumption branch September 23, 2013 14:53
@darobin
Copy link
Member

darobin commented Sep 23, 2013

This LGTM, thanks!

@zcorpan Note that the important thing here is that it matches the Anolis settings used by the HTML spec; so long as we don't change that it works. (And given the styling we have, it seems unlikely that we'd change anything).

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.

3 participants