-
Notifications
You must be signed in to change notification settings - Fork 310
Reimplement associated techniques as data #4509
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for wcag2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
c1f5284
to
a04e41f
Compare
TF members are asked to review the differences in wording within list items, etc., that will result from this (Covered by Changes in... headings in prior comment). |
@dbjorge expressed concern at the one-data-file approach, from the standpoint of potential confusion among authors/editors/reviewers, and advocated for storing the data as frontmatter in each existing HTML file. @alastc had also initially asked if keeping the associations within each HTML file would be possible. While it is possible, the ergonomics around editing would be significantly worse. Here's a breakdown of the 3 possibilities: Option 1 (current): All in one JS file
Option 2: Frontmatter in each HTML file(I investigated this option very briefly; the cons listed below immediately discouraged me from an authoring perspective, which was a big factor considering I authored the entire 1400-line data file by hand, and there was no way I was doing that without allowing my IDE to help.)
Option 3:
|
I am all for making things easier for casual would-be contributors.
I agree that not being able to work on singleton files is a barrier for would-be contributors.
Unfortunately, that's my lived experience with WCAG2ICT and a few other repos. It's not great, but it's not been terrible enough to motivate me to fork a local build.
I don't think so. I am interested in learning more about the WCAG3 workflow, but maybe after that gets more shaken out. |
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.
Discussed in 1-1 and I am content with this approach given the options.
My review: 1.2.2: I'm fine with this, but wanted to emphasize that this is not the same as what you did in 4.1.2. First one uses "one or more" and the other "any". I think I understand the rationale; just pointing out the phrase swap is not consistent. As well, here you have merged two separate listings of G87 (which I think is fine). It's a little unusual having a technique bullet with no technique -- but we do it elsewhere; maybe we can eventually get a technique written? I do think we should consider a tweak to the language, changing "that has" to 'with". The rationale is that in most circumstances, it is the author who supplies the video player:
4.1.2: I support modify the phrase "using technology-specific techniques below" with "using one or more of the following techniques" 1.2.3: I think this should also be "one or more", just like you did with 4.1.2. For instance, one could both add narration in the soundtrack and add audio descriptions. 1.2.5 & 1.2.7: Your change is fine. I think we should also strip out the unlinked generic bullet for same reasons as 1.2.3 3.3.2 & 4.1.3: Your change is fine. 1.2.4: I already commented on this in 1.2.2. I think the same tweak to wording is appropriate here. 1.4.8: This is a little unusual. I agree with taking out the "OR" but I wonder if we should tweak the Instructions to read:
2.1.3: Fine. 2.2.6: It looks like all that's changed is removing the terminal punctuation. Although we do normally have no period at the ends of the bulleted technique items, it's a little odd in this situation, because there is no preamble, and no link. I can live with it. 2.5.5: Fine (and removing periods okay too, for consistency). 3.3.3: Fine. |
Responding to @mbgower; I am suggesting separating a few comments into their own issue/PR when they go beyond migrating the existing content and might prompt further discussion. I'd rather avoid needing to track multiple big threads within this already-big PR, and don't want to bury editorial changes in this PR that would clearly go beyond migrating what currently exists.
I'm a bit reluctant to make the common "using" phrase even wordier unless we really think it's warranted (given it's existed as-is for years), and I'd want to run this by the rest of the TF. It might spawn enough discussion to warrant a separate issue/PR.
1.2.2's original wording explicitly included "any"; 4.1.2's did not, but would have defaulted to just "one", and the intended plurality was unclear to me. I could certainly switch 4.1.2 to "any" as well if we think it works better there.
Interesting point; this goes beyond porting the existing phrasing, and I think this could potentially spawn enough discussion to warrant its own separate PR.
This is asking for changes beyond porting what existed. The wording for 1.2.3 remains completely intact in this PR (including the wording of exactly "one"). I think this would warrant a separate PR.
You are changing "one" to "one or more" in the process, which contradicts the original text (and the "OR"s we are removing). You're also shifting "numbered" from referring to the items (which I have to assume alluded to the technique IDs, unless this was actually an How about: "you must satisfy each requirement using one of its listed techniques"?
The verb conjugation at the beginning of each item also changed. |
a04e41f
to
c8c2763
Compare
Briefly discuss on backlog call, many concerns can/will be addressed as new Issues/PR created by @kfranqueiro after this is merged. |
This replaces the HTML for each Understanding page's Techniques section with a data representation of the same information, to improve consistency, reduce likelihood of mistakes, and eventually enable restoring the structured
techniques
field to success criteria in the JSON export (see the first list item in #4393).All associated techniques are now defined in
understanding/understanding.11tydata.js
under theassociatedTechniques
field, in a way that should make the simplest cases easier to define than they were before. Documentation is added in this PR. In addition, various common mistakes are checked for at runtime with specific error messages, and typings are also defined, enabling some amount of autocomplete and feedback in editors that support TypeScript, such as Visual Studio Code.Most of the changes in this PR involve the build system and centralized templates; the only changes to the Understanding pages themselves are to use the new template rather than spell out the related techniques within HTML.
However, this does visibly impact some pages within the informative docs. I have outlined notable changes to focus TF/WG review on below.
Changes to Understanding pages
This list describes changes to the Techniques section within respective SC pages.
Changes to Techniques pages
This list describes changes within the About this Technique box, which represents the same relationships as the SC pages, but in reverse.