Closed
Conversation
dbjorge
requested changes
Nov 26, 2024
Contributor
dbjorge
left a comment
There was a problem hiding this comment.
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.
doc/issue_impact.md
Outdated
| ### 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. |
Contributor
There was a problem hiding this comment.
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
| Plugins currently support two functions: a "run" function and a "collect" function. Together these functions can be combined to implement complex behaviors on top of the axe system. | ||
|
|
||
| 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: |
Contributor
There was a problem hiding this comment.
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: |
doc/release-and-support.md
Outdated
| Additionally, we recommend that you always use the latest patch version of whichever minor version you are on. For example if you are using axe-core 3.5.5, and 3.5.6 is released it is best to upgrade immediately. Patch releases of axe-core should not find new issues, although they occasionally resolve issues in the case of false positives. | ||
|
|
||
| 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). |
Contributor
There was a problem hiding this comment.
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). |
doc/run-partial.md
Outdated
| **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. |
Contributor
There was a problem hiding this comment.
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. |
| - `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`). |
Contributor
There was a problem hiding this comment.
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.
package-lock.json
Outdated
Contributor
There was a problem hiding this comment.
This should be omitted from the PR
test/node/package-lock.json
Outdated
Contributor
There was a problem hiding this comment.
This should be omitted from the PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
<< Describe the changes >>
Several documentation updates.