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

html-page-title: minimise the number of notes #1340

Merged
merged 5 commits into from
Jun 30, 2020
Merged

Conversation

WilcoFiers
Copy link
Member

This was a suggestion from the ACT TF.

Need for Final Call: none

Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

I'm a bit uncomfortable with the whole crusade for killing notes we've had recently.
I understand that this is orders from the TF and thus we probably don't have that much saying in it… But I think we should have discussed it in a CG call before starting to aggressively remove every single note.
There is an agenda item for this (#1305) but as far as I remember, we've never actually discussed it in a call…

jeeyyy
jeeyyy previously requested changes Jun 10, 2020
@@ -3,7 +3,7 @@ id: 2779a5
name: HTML page has title
rule_type: atomic
description: |
This rule checks that an HTML page has a title.
This rule checks that a non-embedded HTML page has a title. HTML pages embedded into other documents, such as through `iframe` or `object` elements are not applicable because they are not web pages according to the definition in WCAG.
Copy link
Collaborator

Choose a reason for hiding this comment

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

a reference or a definition of what is a non-embedded page will be useful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I'd probably use "top-level browsing context" (or something in that line) instead of "non-embedded", given that it is the technical term of what we try to do. And similarly "nested browsing context" instead of "embedded".

OTOH, so far we've avoided links inside the description of rules.
This was fairly easy when descriptions were short; but might become increasingly harder with longer description (absorbing notes).

I do not know why we were avoiding links in descriptions and whether that still need to be the case or not…

Copy link
Member Author

Choose a reason for hiding this comment

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

I think if we want to do that we should make the description a heading in the doc, instead of making it part of the frontmatter. That's a separate PR. I don't think that should hold up this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking back at that specific case: how would you feel about moving this second sentence to the background section?
(#1305 (comment))

I feel that it is "information about the background for the development of the rule" (to quote the ACT format) and thus fits there.
Putting it in the description feels like it is very important information that we want to show immediately to readers. But in that case, it feels more like some "corner case FAQ" for people having read the rule and wondering "why only non-embedded?"

Jym77
Jym77 previously requested changes Jun 15, 2020
@@ -39,23 +39,17 @@ htmlHintIgnore:

The root element of the [web page](https://www.w3.org/TR/WCAG21/#dfn-web-page-s), if it is an `html` element.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to update that to use https://act-rules.github.io/glossary/#web-page-html instead. This would remove the need for "if it is an html element" since our definition is only about HTML.

This is most likely topic for another PR, however. But may be related to https://github.com/act-rules/act-rules.github.io/pull/1340/files#r438130594

Copy link
Member Author

Choose a reason for hiding this comment

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

Go for it.

@@ -3,7 +3,7 @@ id: 2779a5
name: HTML page has title
rule_type: atomic
description: |
This rule checks that an HTML page has a title.
This rule checks that a non-embedded HTML page has a title. HTML pages embedded into other documents, such as through `iframe` or `object` elements are not applicable because they are not web pages according to the definition in WCAG.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I'd probably use "top-level browsing context" (or something in that line) instead of "non-embedded", given that it is the technical term of what we try to do. And similarly "nested browsing context" instead of "embedded".

OTOH, so far we've avoided links inside the description of rules.
This was fairly easy when descriptions were short; but might become increasingly harder with longer description (absorbing notes).

I do not know why we were avoiding links in descriptions and whether that still need to be the case or not…

_rules/html-page-title-2779a5.md Outdated Show resolved Hide resolved
Co-authored-by: Jean-Yves Moyen <jym@siteimprove.com>
@WilcoFiers WilcoFiers requested review from Jym77 and jeeyyy June 15, 2020 11:57
@WilcoFiers WilcoFiers dismissed stale reviews from Jym77 and jeeyyy June 15, 2020 11:57

Updated

Jym77
Jym77 previously requested changes Jun 16, 2020
@@ -3,7 +3,7 @@ id: 2779a5
name: HTML page has title
rule_type: atomic
description: |
This rule checks that an HTML page has a title.
This rule checks that a non-embedded HTML page has a title. HTML pages embedded into other documents, such as through `iframe` or `object` elements are not applicable because they are not web pages according to the definition in WCAG.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking back at that specific case: how would you feel about moving this second sentence to the background section?
(#1305 (comment))

I feel that it is "information about the background for the development of the rule" (to quote the ACT format) and thus fits there.
Putting it in the description feels like it is very important information that we want to show immediately to readers. But in that case, it feels more like some "corner case FAQ" for people having read the rule and wondering "why only non-embedded?"

@WilcoFiers WilcoFiers dismissed stale reviews from Jym77 and carlosapaduarte June 17, 2020 11:33

Updated, please check again

Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

👍

@WilcoFiers WilcoFiers merged commit 68dc35c into develop Jun 30, 2020
@WilcoFiers WilcoFiers deleted the WilcoFiers-patch-1 branch June 30, 2020 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants