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 table numbers and captions #994

Merged
merged 3 commits into from
Jul 18, 2018
Merged

Add table numbers and captions #994

merged 3 commits into from
Jul 18, 2018

Conversation

emlun
Copy link
Member

@emlun emlun commented Jul 11, 2018

Fixes #990.


Preview | Diff

@emlun emlun added this to the PropRec milestone Jul 11, 2018
@emlun emlun self-assigned this Jul 11, 2018
@emlun emlun requested review from equalsJeffH and selfissued July 11, 2018 16:24
@emlun
Copy link
Member Author

emlun commented Jul 11, 2018

I recommend activating "hide whitespace changes" in the diff settings while reviewing this.

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

LGTM, thx @emlun! (yet again ;) -- only thing I'd add is that we now have another table in https://w3c.github.io/webauthn/#sctn-authenticator-taxonomy to add a caption to. I merged from master into this branch so that table is here now.

oh, and we do need the new <style> -- i.e., it works except for auto-incrementing the table number? I'm guessing "yes" is the answer.

@emlun
Copy link
Member Author

emlun commented Jul 12, 2018

Ok, I'll do this for the new table too. Thanks!

Yes, the style is needed. Here is how Bikeshed's builtin figure numbering works; without the styles added here, these tables would be captioned and numbered as figures.

Oh, but maybe <table><caption/> is more suitable for this than <figure><table/><figcaption/>; I didn't find that element when I looked around earlier. I'll try changing to that and seeing if that works with Bikeshed; in that case the styles can probably be pruned a bit.

@emlun
Copy link
Member Author

emlun commented Jul 12, 2018

Nope, <table><caption/> requires the caption be placed before the table body.

The max-width style is not necessary for the table numbers, but it prevents the tables from getting oddly offset from the text if viewed on a very wide screen.

@emlun
Copy link
Member Author

emlun commented Jul 12, 2018

See also speced/bikeshed#1314

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

LGTM, thx @emlun !

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Looks OK to me

@emlun emlun merged commit faee219 into master Jul 18, 2018
WebAuthnBot pushed a commit that referenced this pull request Jul 18, 2018
WebAuthnBot pushed a commit that referenced this pull request Jul 18, 2018
@emlun emlun deleted the issue-990-table-captions branch July 18, 2018 17:19
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