8000 Automate accessibility support section processing based on rule's input aspects by carlosapaduarte · Pull Request #37 · act-rules/act-tools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Automate accessibility support section processing based on rule's input aspects #37

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

carlosapaduarte
Copy link
Member

No description provided.

Copy link
@zlayaAvocado zlayaAvocado left a comment

Choose a reason for hiding this comment

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

Let me know if I misunderstand something, this is my first act-rules PR review :)

if (aspect.toLowerCase() in supportKeys) {
body = updateAccessibilitySupportInBody(
body,
supportKeys[aspect.toLowerCase()]

Choose a reason for hiding this comment

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

you are converting aspect.toLowerCase() twice, you can just do it once before the if statement

return body;
}

function updateAccessibilitySupportInBody(

Choose a reason for hiding this comment

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

Is there a chance some other parts of the body will need to be updated? If so, this should be abstracted to 1. Get that part by heading name (e.g. Accessibility Support in this case) 2. Update the part based on the string we need to add

body: string,
support: string
): string {
const supportIndex = body.search(/\n###\s+Accessibility\s+Support\b/m);

Choose a reason for hiding this comment

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

Is it enforced somewhere that if a rule has accessibility support it will be exactly named "Accessibility Support"? Just want to make sure this can't be easily renamed and break this automation, or someone making a typo, e.g. "Acessibility Support"

I think anyway "Accessibility Support" and all other hardcoded strings, e.g. "There are no accessibility support issues known." should be constants, something along the lines

const SECTIONS = {
  ACCESSIBILITY_SUPPORT: {
    HEADING_PATTERN: /\n###\s+Accessibility\s+Support\b/mi,
    HEADING: "Accessibility Support"
    NO_ISSUES_TEXT: "There are no accessibility support issues known."
  }
};

Comment on lines +138 to +140
### Accessibility Support

There are no accessibility support issues known.

Choose a reason for hiding this comment

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

is it always going to be accurately formatted, e.g. with line breaks?

// no support issues, replace with new support issue
return (
body.substring(0, supportIndex) +
`\n### Accessibility Support\n\n${support}\n\n` +

Choose a reason for hiding this comment

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

is it a level 3 heading? I am seeing it as 2 here https://www.w3.org/WAI/standards-guidelines/act/rules/73f2c2/ but let me know if I am looking at the wrong page

@zlayaAvocado
Copy link

@carlosapaduarte Can you also add a PR description (explain how / where it's going to update the accessibility support section), it will help me and any new people understand what this is doing better. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Non 444B e yet
Development

Successfully merging this pull request may close these issues.

2 participants
0