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

Editorial: add valuetype column to ariamixin correspondance table #2105

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

keithamus
Copy link
Member

@keithamus keithamus commented Jan 15, 2024

Refs #1110

While looking through #1110 to figure out what we can do, I started enumerating all of the ariamixing properties and their corresponding value types, which I then realised is probably a good reference to have around, and so I added the value type as an additional column to the table that maps ARIAMixin properties to their corresponding attributes.

This information is duplicated in each of the attributes, but it's a little trickier to hunt for the table and navigate around the spec a lot. This table allows us to more easily see at-a-glance what value types map to what properties in the IDL.


Preview | Diff

<tr><td><dfn>ariaPressed</dfn></td><td><sref>aria-pressed</sref></td></tr>
<tr><td><dfn>ariaReadOnly</dfn></td><td><pref>aria-readonly</pref></td></tr>
<tr><th>IDL Attribute</th><th>Reflected ARIA Content Attribute</th><th>Value type</th></tr>
<tr><td><dfn data-dfn-for="ARIAMixin">role</dfn></td><td><a href="#introroles">role</a></td><td><a href="#valuetype_token_list">token list</a></td></tr>
Copy link
Member Author

Choose a reason for hiding this comment

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

role was actually quite difficult to look up because we don't have a table describing the value type of role, and also the reference was missing (so I've added it here).

@spectranaut spectranaut requested review from spectranaut and removed request for jnurthen January 18, 2024 18:08
@cookiecrook cookiecrook self-requested a review January 18, 2024 18:09
Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

Hi @keithamus ! I really appreciate you looking into this, a few requests. Also, maybe the html "types" should be included here as well, for ease of reference.

common/acknowledgements/aria-wg-active.html Outdated Show resolved Hide resolved
common/acknowledgements/aria-wg-participants.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@keithamus
Copy link
Member Author

Also, maybe the html "types" should be included here as well, for ease of reference.

@spectranaut shall I do that in this PR or a follow up?

@spectranaut
Copy link
Contributor

spectranaut commented Jan 29, 2024

Also, maybe the html "types" should be included here as well, for ease of reference.

@spectranaut shall I do that in this PR or a follow up?

If you also think it would be useful, then I would recommend doing it in this PR. It's a little repetitive of the previous section but personally I think repetitivity in specs -- if it helps the consumer of the spec -- is fine.

Would be nice to hear an editor weigh in though, @pkra ? :)

@pkra
Copy link
Member

pkra commented Jan 30, 2024

@spectranaut @keithamus editorially this change looks good to me.

As an aside. It made me wonder if this table should be automatically generated, i.e., moving the IDL Attribute into the characteristics tables and have aria.js create the table. But that would have to wait until aria.js is in better shape.

@cookiecrook
Copy link
Contributor

@annevk

index.html Outdated Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Looks good modulo nits.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
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.

None yet

5 participants