Skip to content

Escape examples for role="time"#1638

Merged
jnurthen merged 4 commits into
w3c:mainfrom
saschanaz:time-element
Nov 18, 2021
Merged

Escape examples for role="time"#1638
jnurthen merged 4 commits into
w3c:mainfrom
saschanaz:time-element

Conversation

@saschanaz

@saschanaz saschanaz commented Nov 4, 2021

Copy link
Copy Markdown
Member

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

pkra commented Nov 5, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
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

pkra commented Nov 5, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
Member Author

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

@scottaohara scottaohara left a comment

Copy link
Copy Markdown
Member

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

pkra commented Nov 5, 2021

Copy link
Copy Markdown
Member

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

pkra commented Nov 18, 2021

Copy link
Copy Markdown
Member

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