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

fixes #81: support ins/del diff styles for table rows #82

Merged
merged 3 commits into from
Apr 21, 2016

Conversation

michaelficarra
Copy link
Member

Fixes #81. 🚣 🌈

@michaelficarra
Copy link
Member Author

I added support for lists as well. Some related information from the HTML specification (basically saying ins/del are incompatible with tables and lists):

@domenic
Copy link
Member

domenic commented Mar 18, 2016

-1. You should not mark these up with unsemantic .del and .ins classes. You should use <ins> and <del> on the contents of the table. See e.g. https://domenic.github.io/zones/#monkeypatch-realms

@michaelficarra
Copy link
Member Author

@domenic Sorry, but HTML's not expressive enough for us here (without potentially some huge hoop-jumping). You simply can't reproduce the same aesthetic with <ins> in every <td>. I would be okay with the compromise of using the ins class on the <tr> for styling, putting an <ins> in every <td> (so we still have semantic markup), and disabling styles for tr.ins > td > ins.

edit: See my usage here for comparison: http://tc39.github.io/Function-prototype-toString-revision/#sec-ecmascript-function-objects

@domenic
Copy link
Member

domenic commented Mar 18, 2016

I don't understand why your usage can't just use my technique.

@michaelficarra
Copy link
Member Author

@domenic I'm trying to encode the information that the entire row was added, as well as allow ecmarkup to style that row in a way that makes it clear to humans. See the difference in appearance between my linked example and yours. I can't do that without a class on the <tr> because of HTML restrictions in the structure of <table>s (and <ul> and <ol>).

@domenic
Copy link
Member

domenic commented Mar 18, 2016

I think it's pretty clear that in my example an entire row was added.

@michaelficarra
Copy link
Member Author

@domenic I've added the style reset to <ins> within tables/lists, so use of this class is entirely optional. Using individual <ins> to mark up semantic insertion will still be encouraged, though proposal authors will also be able to mark a row or list item as being inserted. I think this patch now provides strictly additive value.

@bterlson
Copy link
Member

Sorry I didn't see a notification for this PR anywhere :( Am I even watching my own repo??

Anyway, I think this is fine. Long term I hope diff tools like aria's will make manual ins/del less used. But for now it seems ok to add some authoring conveniences even though it's not as ideal as it could be.

@bterlson bterlson merged commit c0784ff into tc39:master Apr 21, 2016
@michaelficarra michaelficarra deleted the patch-1 branch April 21, 2016 17:16
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