Skip to content

Conversation

@saschanaz
Copy link
Member

@saschanaz saschanaz commented Nov 4, 2021

Fixes another issue from #1623

Not sure whether using role="time" in the definition of role="time" makes sense, but please tell me if it actually does.


Preview | Diff

@pkra
Copy link
Member

pkra commented Nov 5, 2021

The line above the list reads:

Examples of valid date- or time-related strings as text contents of an element with the time role:

I think the change would make this less clear. Readers would need to know the time element's implicit role.

On the other hand, these examples lack an example wrapper (class="example") and the section could probably use some work in general.

@saschanaz
Copy link
Member Author

<time> will not be visible as-is to readers, unless you mean screen readers, which are expected to understand <time> by now if I'm not wrong?

@pkra
Copy link
Member

pkra commented Nov 5, 2021

Right, sorry for having been unclear. I mixed my first and second point.

I find this whole "example" confusing. Usually, examples in the spec are marked up separately and present code samples.

If this is supposed to be such a code example, then I'm not sure what it's covering. If it's just about strings, there's no need for any markup. If it's about role=time markup, the current markup seems better -- but needs to be escaped.

Not sure I'm making more sense now.

@saschanaz
Copy link
Member Author

Escaping sounds good so I just did that, hope that's more clear now.

Copy link
Member

@scottaohara scottaohara left a comment

Choose a reason for hiding this comment

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

thanks @saschanaz, this helps!

edit: nm on that editorial issue. bigger things to worry about.

@saschanaz saschanaz changed the title Use <time> instead of role="time" for now Escape examples for role="time" Nov 5, 2021
@pkra
Copy link
Member

pkra commented Nov 5, 2021

Thanks @saschanaz !

@jnurthen jnurthen self-requested a review November 11, 2021 18:16
@jnurthen jnurthen added the editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo label Nov 11, 2021
@pkra
Copy link
Member

pkra commented Nov 18, 2021

ping @jnurthen

@jnurthen jnurthen merged commit 280b8f4 into w3c:main Nov 18, 2021
jnurthen pushed a commit that referenced this pull request Nov 18, 2021
* escape
* aside class=example
@saschanaz saschanaz deleted the time-element branch November 18, 2021 23:38
@saschanaz saschanaz mentioned this pull request Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants