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: make shape attributes table consistent #9851

Conversation

keithamus
Copy link
Contributor

@keithamus keithamus commented Oct 12, 2023

This formats the wording & HTML around shape attribute such that it is consistent with the format described in #9832


/image-maps.html ( diff )

@keithamus keithamus force-pushed the editorial-make-shape-attributes-table-consistent branch from e85037c to 1334934 Compare October 12, 2023 08:50
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.

This is also not ideal as-is I think. Non-conforming is a state that for this particular attribute is relevant for https://html.spec.whatwg.org/#canonical-keyword as multiple keywords map to the same state. And non-conforming is not the same as a brief description.

We could restate the non-conforming keywords after the table, but perhaps having an additional column for them is better. Some kind of "Non-conforming" column with checkmarks. Or perhaps the inverse if folks think that's not clear enough.

@domenic
Copy link
Member

domenic commented Oct 12, 2023

As I mentioned in #9832 (comment), I think we should keep the notes column, in addition to adding the brief description column.

@keithamus keithamus force-pushed the editorial-make-shape-attributes-table-consistent branch 2 times, most recently from 56d6413 to ce3bb26 Compare October 16, 2023 08:25
@keithamus
Copy link
Contributor Author

keithamus commented Oct 16, 2023

Okay I've kept the Notes column, but given how State is now on the right, it felt unclear to have Notes anywhere but adjacent to the Keyword column, so it now reads Keywword, Notes, State, Brief Description, which is a little odd.

Let me know your thoughts and I'll update #9850 to do the same.

@annevk
Copy link
Member

annevk commented Oct 16, 2023

@domenic how about we instead move the "non-conforming" annotations into the keyword column? As

circ (non-conforming)

? (And then drop the paragraph that states they are non-conforming as it's redundant with the table.)

@domenic
Copy link
Member

domenic commented Oct 17, 2023

It looks pretty weird that way:
image

This is maybe slightly better?

image

Not sure what to suggest...

I agree the paragraph is redundant and can be removed.

@annevk
Copy link
Member

annevk commented Oct 17, 2023

From those two tables I think the first is better a11y-wise. I'd be a bit worried that if you land on one of the non-conforming cells in isolation you can't make sense of it, unless perhaps we'd also mark the keywords as headings?

(Amazing by the way how it's inconsistent with regards to whether the abbreviated keyword is conforming or not.)

@domenic
Copy link
Member

domenic commented Oct 17, 2023

We could always overkill it with CSS grid inside the table cell, at the cost of some extra markup and having to edit the stylesheet...

@domenic
Copy link
Member

domenic commented Nov 16, 2023

Separate Conforming column:
image

@domenic
Copy link
Member

domenic commented Nov 16, 2023

The CSS right-aligning ended up looking weird to my eyes, but maybe if I was better at design it would be better...
image

@domenic
Copy link
Member

domenic commented Nov 16, 2023

In Chat we have settled on the extra Conforming column. Notes:

  • Add class="yesno" to the table
  • Add class="no" to the "no" cells to get them centered.
  • Don't do anything for the yes cells, IMO?
  • No more need for the "Note the circ, polygon, and rectangle, keywords are non-conforming." sentence.
  • Separately, "default" is missing a "Brief description".

@keithamus keithamus force-pushed the editorial-make-shape-attributes-table-consistent branch from ce3bb26 to b5c2171 Compare November 18, 2023 11:19
@keithamus
Copy link
Contributor Author

keithamus commented Nov 18, 2023

I've added the classes but the styles aren't applying as /styles.css seems to be the wrong URL. Is this a known issue?

Aside from that I think this is complete.

@domenic
Copy link
Member

domenic commented Nov 18, 2023

Yeah, PR preview doesn't seem to work that well in certain ways. One of us can do a local build and double-check before merging (probably during the workweek).

source Show resolved Hide resolved
source Show resolved Hide resolved
@keithamus keithamus force-pushed the editorial-make-shape-attributes-table-consistent branch from b5c2171 to 3ddfa50 Compare November 20, 2023 09:50
@domenic domenic merged commit 3bfb5a8 into whatwg:main Nov 21, 2023
1 check passed
@keithamus keithamus deleted the editorial-make-shape-attributes-table-consistent branch November 21, 2023 09:11
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.

3 participants