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

Wrap heading section numbers in <span class=secno> #44

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

sideshowbarker
Copy link
Member

Addresses #27.

@domenic
Copy link
Member

domenic commented Apr 18, 2017

Ah, sorry for not updating #27, but in #30 (comment) @annevk mentioned he'd prefer we only output this in the dev edition. I'm not sure that's a great idea personally---I can imagine our future selves getting very confused about the divergence in markup between the two while tweaking some stylesheets---but that's where we landed last time. @annevk, thoughts?

@annevk
Copy link
Member

annevk commented Apr 18, 2017

I'm somewhat indifferent at this point. My main quibble is that the generated HTML of Bikeshed is way out of proportion to what it could be I think so I'd rather not do anything just to match that, but if this helps make progress just go ahead. It's a larger issue anyway.

@sideshowbarker
Copy link
Member Author

in #30 (comment) @annevk mentioned he'd prefer we only output this in the dev edition. I'm not sure that's a great idea personally---I can imagine our future selves getting very confused about the divergence in markup between the two while tweaking some stylesheets

If we do decide we only need it for the dev edition, I’m pretty sure it’s possible (maybe even easy) to make wattsi only do it for the dev edition

@domenic
Copy link
Member

domenic commented Apr 18, 2017

Let's merge this as-is then; although I understand @annevk's concerns in general, and applied them in #30 by removing my addition of Bikeshed's heading class, I feel more comfortable having the same markup structure between the two editions.

@domenic domenic merged commit 60257c5 into master Apr 18, 2017
@domenic domenic deleted the sideshowbarker/add-secno branch April 18, 2017 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants