-
Notifications
You must be signed in to change notification settings - Fork 9
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
Editorial: Replace mapping table and summary/details with headings #180
Conversation
My browser is hanging when opening up the larger commit so I can't leave inline comments. Here are some things I noticed so far
@spectranaut @jnurthen I'm not sure how to make sure that nothing was accidentally dropped. Do you have any thoughts on that? |
Do you mean in the PR Preview? The styles have always been broken here, unfortunately. You can download and look at it locally or use githack: https://raw.githack.com/w3c/core-aam/remove-tables/index.html#mapping_role I wonder if there is a way to fix this.
This is how it was before, but we can change it. The "repeat" was a link to ARIA, but now it seems broken, as you point out below.
I'll fix this, it's weird though.. those used to be links back to the ARIA spec. But even in the editors draft they link just back to core-aam, I'm not sure how or when this changed.
I wrote a little script to do the translation locally, I'm pretty confident I didn't drop anything! I'd guess we don't have to worry about that. |
Thanks for clarifying, @spectranaut! (Especially kindly forgiving me for forgetting about the CSS issue). This looks good to me then. |
FYI the PR preview issue seems to be tobie/pr-preview#87 |
As per ARIA Editors meeting 2023-06-26, @scottaohara wants to coordinate with @spectranaut on this. |
spent time checking this out and while i had mentioned during the editor's call that maybe we could revise the presentation/markup for this component, I'm going to retract those ideas for now. No reason to hold this up further, and i'm doubtful the benefit of changing this up further at this point. i'm assuming the broken links to the ARIA spec will be fixed once this gets merged? The links are setup exactly the same between the published version and this, so I can only assume it's just some problem with respec not building properly? |
As per ARIA Editors meeting 2023-07-10, @spectranaut will simplify further, dropping summary/details, introducing headings etc. |
The HTML is a little wacky, I'll fix before landing, but @scottaohara and @pkra and @jnurthen what do you think of this radical change we discussed: https://raw.githack.com/w3c/core-aam/remove-tables/index.html#mapping_role |
i think it's an improvement. tbh i'm going back and forth on whether the individual roles/attributes should take part in the TOC... but it would definitely help with the easier permalinking |
I'll second @scottaohara -- looks much better to me, too! Could we drop the table captions as well if we linked the terms in the headings? For roles, we could alternatively just link the computed role entry but we don't have that for states and properties (though I suppose that might happen if we had synonymous properties some day). |
I was also trying to think of a way to drop the headings. Maybe I'll just add an extra row to all the tables and say "WAI-ARIA reference" in the first column and the relevant linked role or attribute in the second? |
|
That's arguably clearer anyway. Right now, unless you know how the spec is structured, you might not necessarily know what that link is for, since it effectively duplicates the heading label. |
bcb2f9c
to
048070c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spectranaut i think this is good to go.
optional change would be to update the spec to not include the individual role/attribute names in the TOC. but that doesn't need to be part of this change, if it's done at all
048070c
to
cdec3fe
Compare
Githack link, as PR Preview doesn't include styles: https://raw.githack.com/w3c/core-aam/remove-tables/index.html#mapping_role
Preview | Diff