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

feat(rule): Inline text spacing must be adjustable with custom stylesheets #1446

Merged
merged 7 commits into from
Apr 17, 2019

Conversation

jeeyyy
Copy link
Contributor

@jeeyyy jeeyyy commented Mar 19, 2019

Ensure text spacing is not affected by inline spacing styles that affects CSS specificity

Closes issue:

Note: to make sure to make it clear in the failure message that this is due to limited support for custom stylesheets.

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: Steve

@jeeyyy jeeyyy requested a review from a team as a code owner March 19, 2019 16:55
@jeeyyy jeeyyy changed the title feat: initial iimplementation Rule: avoid inline spacing Mar 19, 2019
@jeeyyy jeeyyy changed the title Rule: avoid inline spacing feat (rule): avoid inline spacing Mar 19, 2019
@stephenmathieson stephenmathieson changed the title feat (rule): avoid inline spacing feat(rule): avoid inline spacing Mar 19, 2019
straker
straker previously approved these changes Mar 21, 2019
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

LGTM

@jeeyyy jeeyyy requested a review from WilcoFiers April 4, 2019 11:26
@jeeyyy
Copy link
Contributor Author

jeeyyy commented Apr 4, 2019

@straker as you have approved this, you will have to fill out the review checks above.

lib/checks/shared/avoid-inline-spacing.json Outdated Show resolved Hide resolved
"impact": "serious",
"messages": {
"pass": "No inline styles that affect text spacing has been specified",
"fail": "Inline styles that affect text spacing has been specified"
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to include what style props are failing. Something like:

Remove !important from inline styles {= data =}, as overriding this not supported by most browsers

lib/rules/avoid-inline-spacing.json Outdated Show resolved Hide resolved
test/checks/shared/avoid-inline-spacing.js Show resolved Hide resolved
lib/checks/shared/avoid-inline-spacing.js Outdated Show resolved Hide resolved
lib/rules/avoid-inline-spacing.json Outdated Show resolved Hide resolved
"tags": ["wcag21", "wcag1412"],
"metadata": {
"description": "Ensure text spacing is not affected by inline spacing styles that affects CSS specificity",
"help": "Remove inline styles with '!important' that affect text spacing, as overriding this is not supported by most browsers"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did a little more editing. I think the pass / fail ones are good. Can you change help/description to:

Description: Ensure that test spacing set through style attributes can be adjusted with custom stylesheets
Help: Inline text spacing must be adjustable with custom stylesheets

The browser thing is part of the fail message, we don't need to repeat that.

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Reviewed for security.

@WilcoFiers WilcoFiers changed the title feat(rule): avoid inline spacing feat(rule): Inline text spacing must be adjustable with custom stylesheets Apr 17, 2019
@WilcoFiers WilcoFiers merged commit 430b07f into develop Apr 17, 2019
@WilcoFiers WilcoFiers deleted the rule-avoid-inline-spacing branch April 17, 2019 10:13
stephenmathieson added a commit to mohanraj-r/axe-core that referenced this pull request May 10, 2019
* develop: (50 commits)
  docs: add jsdom example and tests (dequelabs#1530)
  fix(aria-valid-attr-value): allow aria-owns to pass when element is not in the DOM (dequelabs#1526)
  fix(isSkipLink): cache first page link (dequelabs#1525)
  chore: update Babel dependencies (dequelabs#1527)
  chore(package): Update karma to version 4.1.0 (dequelabs#1528)
  feat: Improve perf of axe.run [WWD-1821] (dequelabs#1503)
  fix(prettier): ignore generated api doc files (dequelabs#1522)
  fix(skip-link,region): Allow multiple skiplinks at page top (dequelabs#1496)
  fix(raw-reporter): do not output `DqElement`s (dequelabs#1513)
  fix: Scroll state had top and left properties flipped (dequelabs#1469)
  refactor: commons.color.getBackgroundColor method (dequelabs#1451)
  fix(aria-valid-attr-value): allow aria-controls to pass when element is not in the DOM
  chore: Update husky to the latest version 🚀 (dequelabs#1514)
  style: format HTML files with Prettier (dequelabs#1508)
  test: Fix invalid test html (dequelabs#1502)
  feat(rule): Inline text spacing must be adjustable with custom stylesheets (dequelabs#1446)
  chore: Remove version number from axe.d.ts (dequelabs#1499)
  chore: Update make-dir to the latest version 🚀 (dequelabs#1465)
  fix: Exclude  iframe for html-has-lang rule (Issue 1424) (dequelabs#1430)
  feat(utils): add support for complex CSS selectors (dequelabs#1494)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants