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

docs: documentation updates #4642

Closed
wants to merge 4 commits into from
Closed

docs: documentation updates #4642

wants to merge 4 commits into from

Conversation

Jeremy-Rivera
Copy link

<< Describe the changes >>

Several documentation updates.

@Jeremy-Rivera Jeremy-Rivera requested a review from a team as a code owner November 20, 2024 22:32
Copy link
Contributor

@dbjorge dbjorge left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good overall! A few suggestions inline.

It'd also be good to update the PR title to something a bit more descriptive; the "docs" prefix already implies that it's a documentation update, something like docs: typo and grammar corrections would be more helpful to someone reviewing the change log later and trying to decide if they want to review the individual change in more detail.


## Definitions

### Minor

Considered to be a nuisance or an annoyance bug. Prioritize fixing if the fix only takes a few minutes and the developer is working on the same screen/feature at the same time, otherwise the issue should not be prioritized. Will still get in the way of compliance if not fixed.
Considered to be a nuisance or an annoyance bug. Prioritize fixing it if the fix only takes a few minutes and the developer is working on the same screen/feature at the same time, otherwise the issue should not be prioritized. This issue will still get in the way of compliance if not fixed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd leave the last sentence here as-is as a sentence fragment for consistency/conciseness with the other text around it. All of these definitions are written in fragment form (all of their first sentences, + some other last sentences).

doc/plugins.md Outdated

In order to create such a plugin, we need to implement the "run" function for the plugin, and the command that registers and executes the "run" function within each iframe on the page that contains axe. Lets look at what a noop implementation of this run function would look like:
To create such a plugin, we need to implement the 'run' function and the command that registers and executes the 'run' function within each iframe on the page containing axe. Let's look at what a noop implementation of this run function would look like:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To create such a plugin, we need to implement the 'run' function and the command that registers and executes the 'run' function within each iframe on the page containing axe. Let's look at what a noop implementation of this run function would look like:
To create such a plugin, we need to implement the `run` function and the command that registers and executes the `run` function within each iframe on the page containing axe. Let's look at what a noop implementation of this run function would look like:


Ensuring you always use the latest available patch version of axe-core on any minor line guarantees you always the most secure version of axe-core. This minor line must have been released within the last 18 months. See [security updates](#security-updates).
Ensuring that you always use the latest available patch version of axe-core guarantees that you are using the most secure version. This minor line must have been released within the last 18 months. See [security updates](#security-updates).
Copy link
Contributor

Choose a reason for hiding this comment

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

The "on any minor line" part is important to keep for meaning here. If our last few releases were 4.9.2, 4.9.3, 4.10.0, 4.10.1, "the latest available patch version" means "4.10.1", but what this paragraph is saying is "even if you're still on the 4.9 minor line, you should ensure that you're using the latest patch version within that minor line (ie, 4.9.3 rather than 4.9.2)"

Suggested change
Ensuring that you always use the latest available patch version of axe-core guarantees that you are using the most secure version. This minor line must have been released within the last 18 months. See [security updates](#security-updates).
Ensuring that you always use the latest available patch version of axe-core on any minor line guarantees that you are using the most secure version. This minor line must have been released within the last 18 months. See [security updates](#security-updates).

@@ -42,7 +42,7 @@ function runPartialRecursive(context, options = {}, win = window) {

**important**: The order in which these methods are called matters for performance. Internally, axe-core constructs a flattened tree when `axe.utils.getFrameContexts` is called. This is fairly slow, and so should not happen more than once per frame. When `axe.runPartial` is called, that tree will be used if it still exists. Since this tree can get out of sync with the actual DOM, it is important to call `axe.runPartial` immediately after `axe.utils.getFrameContexts`.

To run efficiently, `axe.runPartial` calls should happen in parallel, so that when possible browsers can test multiple frames simultaneously.
To run efficiently, `axe.runPartial` calls should happen in parallel, so that, when possible, browsers can test multiple frames simultaneously.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To run efficiently, `axe.runPartial` calls should happen in parallel, so that, when possible, browsers can test multiple frames simultaneously.
To run efficiently, `axe.runPartial` calls should happen in parallel so that, when possible, browsers can test multiple frames simultaneously.

@@ -39,13 +39,13 @@ The [`ariaAttrs`](../lib/standards/aria-attrs.js) object defines valid ARIA attr
- `type` - string(required). The attribute type which dictates the valid values of the attribute. Valid types are:
- `boolean` - Boolean attributes only accept `true` or `false` as valid values (e.g. `aria-selected`).
- `nmtoken` - Name token attributes accept a single value from a list of valid values (e.g. `aria-orientation`).
- `mntokens` - Name tokens attributes accept a space separated list of values from a list of valid values (e.g. `aria-relevant`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's file a separate issue/PR for correcting this (here and on L48); this will also need a corresponding update to our typings and tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be omitted from the PR

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be omitted from the PR

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

Successfully merging this pull request may close these issues.

2 participants