-
Notifications
You must be signed in to change notification settings - Fork 70
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
Text spacing rewrite #1923
Text spacing rewrite #1923
Conversation
Target text nodes rather than elements (+ note that CSS property is actually on parent). |
@WilcoFiers I've updated the |
|
||
## Expectation | ||
- the [specified][] value is [declared][] in a `style` attribute; and | ||
- the [computed][] value is [important][]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the computed value important
?
Or is it the declaration of the letter-spacing
property that is important
?
The important
declaration influences how the computed value is obtained, but I'm not sure we can say the computed value is important
....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. We do need to somehow look at the computed value due to Inapplicable Example 8, where the inherit !important
declaration is important but we don't care because the computed value is the inherited one, coming from a non-important declaration and thus can be overwritten.
I do agree that only declarations seems to be important 🙈
Given that this wording is already present in the current rule (first bullet in Expectation + Passed Example 7), I suggest we treat that in another Issue/PR: #2002
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comment on #2002 on a way to deal with this
Co-authored-by: Carlos Duarte <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a spelling issue
Co-authored-by: Carlos Duarte <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little editorial to the applicabilities, but otherwise I think these look good.
Call for reviews end on October 12. |
Call for review has ended, merging. |
* Add new letter-spacing rule and deprecate old one * Add new word-spacing rule and deprecate old one * Clean up assumptions * Clean up * Clean up * Add new line-height rule and deprecate old one * Replace old letter spacing version rather than deprecating it * Replace old line height version rather than deprecating it * Replace old word spacing version rather than deprecating it * Target text nodes * Improve background note * Apply suggestion from review * Clean up * Target text nodes rather than their parents * Target text nodes rather than their parents * Add missing reference * Update example * Apply to parent of text nodes, not text nodes * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> * Typos Co-authored-by: Carlos Duarte <[email protected]> * Typos --------- Co-authored-by: Carlos Duarte <[email protected]>
…2125) * Update programmatically-determined-link-context.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update programmatically-determined-link-context.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Create modal-dialog * Rename modal-dialog to modal-dialog.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Apply suggestions from code review Co-authored-by: Dan Tripp <[email protected]> * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update and rename modal-dialog.md to inert.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update spelling-ignore.yml * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update inert.md * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Dan Tripp <[email protected]> * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update inert.md * Apply suggestions from code review Co-authored-by: Wilco Fiers <[email protected]> * AGWG Updates (#2067) * Add (alt="") for clarity on empty alt * Resolve focus visible feedback * Tweak contrast rules * Tweak page title descriptive * Fix tests * Apply suggestions from code review Co-authored-by: Trevor R. Bostic <[email protected]> --------- Co-authored-by: Trevor R. Bostic <[email protected]> * [cae760] Frame has non-empty accessible name (#2034) * First pass in response to Feb 16 TF meeting * typo * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Move note about frame to background * Set height for frame * Test wants alphabetical contributors * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Move note to background --------- Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Updating glossary definition. (#2069) * Bump yaml and zx (#2056) * Bump yaml and zx Bumps [yaml](https://github.com/eemeli/yaml) to 2.2.2 and updates ancestor dependency [zx](https://github.com/google/zx). These dependencies need to be updated together. Updates `yaml` from 1.10.2 to 2.2.2 - [Release notes](https://github.com/eemeli/yaml/releases) - [Commits](eemeli/yaml@v1.10.2...v2.2.2) Updates `zx` from 5.3.0 to 7.2.1 - [Release notes](https://github.com/google/zx/releases) - [Commits](google/zx@5.3.0...7.2.1) --- updated-dependencies: - dependency-name: yaml dependency-type: indirect - dependency-name: zx dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> * Trigger CLA? --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jean-Yves Moyen <[email protected]> * new rule: ARIA required ID references exist (#2041) * new rule: ARIA required ID references exist * Address review comments Co-authored-by: Carlos Duarte <[email protected]> * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> * Apply suggestions from code review Co-authored-by: Tom Brunet <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/aria-required-id-references-in6db8.md * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> --------- Co-authored-by: Carlos Duarte <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Tom Brunet <[email protected]> * scrollable element: clarify the title (#2083) * UpdateTableHeaderRule (#2074) * UpdateTableHeaderRule * Update table-header-cell-has-assigned-cells-d0f69e.md * trigger test * Update _rules/table-header-cell-has-assigned-cells-d0f69e.md Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Adds apostrophe to mark the possessive form (#2080) Co-authored-by: Wilco Fiers <[email protected]> * Focus visible rule: Fix typo (#2082) * Contrast rules: Tweak background text (#2090) * Tweak name / description of Scrollable element keyboard (#2092) * Deprecate HTML page lang and xml:lang attributes have matching values (#2086) * Deprecate HTML page lang and xml:lang attributes have matching values * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> --------- Co-authored-by: Carlos Duarte <[email protected]> * Rephrase Applicability (#2079) * Rename file (#2078) * Move secondary requirement texts out of the background (#2060) * Move secondary requirement texts out of the background * Apply suggestions from code review * fix test * Fix failing test * Secondary reqs on ARIA rules * Update all secondary requirements * Typos * Fix failing tests * Update _rules/link-non-empty-accessible-name-c487ae.md Co-authored-by: Trevor R. Bostic <[email protected]> * Tweaked the language some more * Update rule design info for secondary requirements * Fix tests * Apply suggestions from code review Co-authored-by: Kathy Eng <[email protected]> * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Trevor R. Bostic <[email protected]> Co-authored-by: Kathy Eng <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * fix test on secondary requirements (#2102) * fix test on secondary requirements * More assertions * Update _rules/aria-required-id-references-in6db8.md Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Jean-Yves Moyen <[email protected]> * Update dependencies (including act-tools) (#2103) * fix test-assets not getting built right (#2104) * Update element-lang-valid-de46e4.md (#2100) Co-authored-by: Jean-Yves Moyen <[email protected]> * fix the approve-rule action (#2105) * Remove outdated accsupport note (#2111) * "Element with lang attribute has valid language tag" [de46e4]: Updated Failed Examples 4 and 5 to reflect Applicability (#2094) * Update element-lang-valid-de46e4.md Updated Failed examples 4 and 5 to reflect applicability * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/element-lang-valid-de46e4.md Co-authored-by: Dan Tripp <[email protected]> --------- Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Dan Tripp <[email protected]> Co-authored-by: Carlos Duarte <[email protected]> * Text spacing rewrite (#1923) * Add new letter-spacing rule and deprecate old one * Add new word-spacing rule and deprecate old one * Clean up assumptions * Clean up * Clean up * Add new line-height rule and deprecate old one * Replace old letter spacing version rather than deprecating it * Replace old line height version rather than deprecating it * Replace old word spacing version rather than deprecating it * Target text nodes * Improve background note * Apply suggestion from review * Clean up * Target text nodes rather than their parents * Target text nodes rather than their parents * Add missing reference * Update example * Apply to parent of text nodes, not text nodes * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> * Typos Co-authored-by: Carlos Duarte <[email protected]> * Typos --------- Co-authored-by: Carlos Duarte <[email protected]> * "Meta viewport allows for zoom" (b4f0c3): Explicit meaning of 'has' (#1994) * Explicit meaning of 'has' * Improve expectation and examples * Typo * Improve algorithm description Co-authored-by: Carlos Duarte <[email protected]> * Rephrase expectations * Streamline Applicability * Typo * Simplify expectations --------- Co-authored-by: Carlos Duarte <[email protected]> * Map Empty-heading rule to ARIA instead of WCAG (#2120) Co-authored-by: Jean-Yves Moyen <[email protected]> * Deprecate 4.1.1 rules (#2117) Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: giacomo-petri <[email protected]> Co-authored-by: Carlos Duarte <[email protected]> Co-authored-by: Dan Tripp <[email protected]> Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Trevor R. Bostic <[email protected]> Co-authored-by: Tom Brunet <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: HelenBurge <[email protected]> Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Daniel Montalvo <[email protected]> Co-authored-by: Kathy Eng <[email protected]>
* Add new letter-spacing rule and deprecate old one * Add new word-spacing rule and deprecate old one * Clean up assumptions * Clean up * Clean up * Add new line-height rule and deprecate old one * Replace old letter spacing version rather than deprecating it * Replace old line height version rather than deprecating it * Replace old word spacing version rather than deprecating it * Target text nodes * Improve background note * Apply suggestion from review * Clean up * Target text nodes rather than their parents * Target text nodes rather than their parents * Add missing reference * Update example * Apply to parent of text nodes, not text nodes * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> * Typos Co-authored-by: Carlos Duarte <[email protected]> * Typos --------- Co-authored-by: Carlos Duarte <[email protected]>
…2125) * Update programmatically-determined-link-context.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update programmatically-determined-link-context.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Create modal-dialog * Rename modal-dialog to modal-dialog.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Apply suggestions from code review Co-authored-by: Dan Tripp <[email protected]> * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update and rename modal-dialog.md to inert.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update spelling-ignore.yml * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update inert.md * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Dan Tripp <[email protected]> * Update iframe-not-focusable-has-no-interactive-content-akn7bn.md * Update inert.md * Apply suggestions from code review Co-authored-by: Wilco Fiers <[email protected]> * AGWG Updates (#2067) * Add (alt="") for clarity on empty alt * Resolve focus visible feedback * Tweak contrast rules * Tweak page title descriptive * Fix tests * Apply suggestions from code review Co-authored-by: Trevor R. Bostic <[email protected]> --------- Co-authored-by: Trevor R. Bostic <[email protected]> * [cae760] Frame has non-empty accessible name (#2034) * First pass in response to Feb 16 TF meeting * typo * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Wilco Fiers <[email protected]> * Move note about frame to background * Set height for frame * Test wants alphabetical contributors * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/iframe-non-empty-accessible-name-cae760.md Co-authored-by: Jean-Yves Moyen <[email protected]> * Move note to background --------- Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Updating glossary definition. (#2069) * Bump yaml and zx (#2056) * Bump yaml and zx Bumps [yaml](https://github.com/eemeli/yaml) to 2.2.2 and updates ancestor dependency [zx](https://github.com/google/zx). These dependencies need to be updated together. Updates `yaml` from 1.10.2 to 2.2.2 - [Release notes](https://github.com/eemeli/yaml/releases) - [Commits](eemeli/yaml@v1.10.2...v2.2.2) Updates `zx` from 5.3.0 to 7.2.1 - [Release notes](https://github.com/google/zx/releases) - [Commits](google/zx@5.3.0...7.2.1) --- updated-dependencies: - dependency-name: yaml dependency-type: indirect - dependency-name: zx dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> * Trigger CLA? --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jean-Yves Moyen <[email protected]> * new rule: ARIA required ID references exist (#2041) * new rule: ARIA required ID references exist * Address review comments Co-authored-by: Carlos Duarte <[email protected]> * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> * Apply suggestions from code review Co-authored-by: Tom Brunet <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/aria-required-id-references-in6db8.md * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> --------- Co-authored-by: Carlos Duarte <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Tom Brunet <[email protected]> * scrollable element: clarify the title (#2083) * UpdateTableHeaderRule (#2074) * UpdateTableHeaderRule * Update table-header-cell-has-assigned-cells-d0f69e.md * trigger test * Update _rules/table-header-cell-has-assigned-cells-d0f69e.md Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * Adds apostrophe to mark the possessive form (#2080) Co-authored-by: Wilco Fiers <[email protected]> * Focus visible rule: Fix typo (#2082) * Contrast rules: Tweak background text (#2090) * Tweak name / description of Scrollable element keyboard (#2092) * Deprecate HTML page lang and xml:lang attributes have matching values (#2086) * Deprecate HTML page lang and xml:lang attributes have matching values * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> --------- Co-authored-by: Carlos Duarte <[email protected]> * Rephrase Applicability (#2079) * Rename file (#2078) * Move secondary requirement texts out of the background (#2060) * Move secondary requirement texts out of the background * Apply suggestions from code review * fix test * Fix failing test * Secondary reqs on ARIA rules * Update all secondary requirements * Typos * Fix failing tests * Update _rules/link-non-empty-accessible-name-c487ae.md Co-authored-by: Trevor R. Bostic <[email protected]> * Tweaked the language some more * Update rule design info for secondary requirements * Fix tests * Apply suggestions from code review Co-authored-by: Kathy Eng <[email protected]> * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Trevor R. Bostic <[email protected]> Co-authored-by: Kathy Eng <[email protected]> Co-authored-by: Jean-Yves Moyen <[email protected]> * fix test on secondary requirements (#2102) * fix test on secondary requirements * More assertions * Update _rules/aria-required-id-references-in6db8.md Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Co-authored-by: Jean-Yves Moyen <[email protected]> * Update dependencies (including act-tools) (#2103) * fix test-assets not getting built right (#2104) * Update element-lang-valid-de46e4.md (#2100) Co-authored-by: Jean-Yves Moyen <[email protected]> * fix the approve-rule action (#2105) * Remove outdated accsupport note (#2111) * "Element with lang attribute has valid language tag" [de46e4]: Updated Failed Examples 4 and 5 to reflect Applicability (#2094) * Update element-lang-valid-de46e4.md Updated Failed examples 4 and 5 to reflect applicability * Apply suggestions from code review Co-authored-by: Jean-Yves Moyen <[email protected]> * Update _rules/element-lang-valid-de46e4.md Co-authored-by: Dan Tripp <[email protected]> --------- Co-authored-by: Jean-Yves Moyen <[email protected]> Co-authored-by: Dan Tripp <[email protected]> Co-authored-by: Carlos Duarte <[email protected]> * Text spacing rewrite (#1923) * Add new letter-spacing rule and deprecate old one * Add new word-spacing rule and deprecate old one * Clean up assumptions * Clean up * Clean up * Add new line-height rule and deprecate old one * Replace old letter spacing version rather than deprecating it * Replace old line height version rather than deprecating it * Replace old word spacing version rather than deprecating it * Target text nodes * Improve background note * Apply suggestion from review * Clean up * Target text nodes rather than their parents * Target text nodes rather than their parents * Add missing reference * Update example * Apply to parent of text nodes, not text nodes * Apply suggestions from code review Co-authored-by: Carlos Duarte <[email protected]> * Typos Co-authored-by: Carlos Duarte <[email protected]> * Typos --------- Co-authored-by: Carlos Duarte <[email protected]> * "Meta viewport allows for zoom" (b4f0c3): Explicit meaning of 'has' (#1994) * Explicit meaning of 'has' * Improve expectation and examples * Typo * Improve algorithm description Co-authored-by: Carlos Duarte <[email protected]> * Rephrase expectations * Streamline Applicability * Typo * Simplify expectations --------- Co-authored-by: Carlos Duarte <[email protected]> * Map Empty-heading rule to ARIA instead of WCAG (#2120) Co-authored-by: Jean-Yves Moyen <[email protected]> * Deprecate 4.1.1 rules (#2117) Co-authored-by: Jean-Yves Moyen <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: giacomo-petri <[email protected]> Co-authored-by: Carlos Duarte <[email protected]> Co-authored-by: Dan Tripp <[email protected]> Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Trevor R. Bostic <[email protected]> Co-authored-by: Tom Brunet <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: HelenBurge <[email protected]> Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Daniel Montalvo <[email protected]> Co-authored-by: Kathy Eng <[email protected]>
With #1804, the text spacing rules becomes way too complex.
This simplifies them by looking at the text directly rather than at the
style
attribute. This automatically solves all the problems about inheritance, changed values, … by making sure that we only look at values that are actually used.I feel this version is much simpler even than the current version.
font-size
isn't changed through::first-line
or other, to other rules.Closes issue(s):
Need for Call for Review:
This will require a 2 weeks Call for Review (new versions of the rules, including heavy changes and simplification)
Pull Request Etiquette
When creating PR:
develop
branch (left side).After creating PR:
Rule
,Definition
orChore
.When merging a PR:
How to Review And Approve