aria-practices
aria-practices copied to clipboard
Proposed Code Guide Changes
Hi, here are some thoughts on changes we could make to the code guide to incorporate newer JS features into them. Normally I would open a PR to propose these, but since it is a wiki page, this seemed like the right place to articulate these first.
I read through the code guide and did some perusing through the current set of examples to get a sense of what the general usage and patterns were and would like to propose adding the following to the JavaScript section of the Code Guide:
- Use
letandconstinstead ofvar - Use
for...ofinstead offorloops that don't rely on an index for their logic - Update class syntax to use class fields (as in, replace
this.foo;withfoo;in class declarations) - Use static methods and blocks for shared utilities specific to an example.
- Standardize on using a global object when creating new functions and classes. A lot of examples do this but some do not and it is not called out in the code guide. (For example, use
const aria = aria || {};)
I think these are good changes and in general we should be eliminating any thing using aria = aria || {};.
Our biggest issue is having a plan to update legacy examples to the new coding practices.
@jongund Glad to hear that, thank you for the feedback. I was actually suggesting we standardize on const aria = aria || {}; in each file since all of these examples are at some point running in the context of a larger page and I thought that would be best to have some encapsulation for classes and helper methods. If you have other idea for how to address that I would be very interested to hear them!
The ARIA Authoring Practices (APG) Task Force just discussed Code guide proposals.
The full IRC log of that discussion
<jugglinmike> Topic: Code guide proposals<jugglinmike> github: https://github.com/w3c/aria-practices/issues/3060
<jugglinmike> Matt_King: I would like to get some people with the right engineering background to review and comment on the proposed changes
<jugglinmike> Matt_King: I know jongund has already commented
<jugglinmike> Matt_King: Our code has two goals. One: to use modern coding practices to keep up-to-date with what people expects, and two: to write in such a way to make the accessibility motivations clear
<jugglinmike> OliverH: This reminds me of the coding practices on StackOverflow
<jugglinmike> jongund: I'm not so sure about the recommendation to use class fields. I prefer the explicitness of using the "this" keyword, but maybe I'm just set in my way
<jugglinmike> OliverH: I think it's about reducing redundancy
<jugglinmike> Matt_King: Sometimes, a little redundancy is helpful because it can be taxing to discern the relevant context from the surrounding code
<jugglinmike> Matt_King: But then again, I'm not writing JavaScript enough to really weigh in, here
<jugglinmike> OliverH: I can research what the recommended way to do this and get back to you next week
<jugglinmike> arigilmore: I agree with what's been said in the issue, leaning more toward what OliverH has been saying. I'll keep an eye on the issue and share my perspective as necessary
<jugglinmike> Matt_King: Great, then we'll keep this on the agenda for next week
Thanks for posting the log in here - I'm currently looking into the official best practices. I'll post my updates in here and if you want I'll present them in the meeting next week!
Here are some of my findings:
"Use let and const instead of var"
Recommendation: ✅ I agree that we should replace / correct snippets of the old style. Using var is ignorant of the consequences and dangerous in many if not most cases.
Relevant code guide sections:
"Use
for...ofinstead offorloops that don't rely on an index for their logic"
Recommendation: ✅ Personally I'd generally recommend using the newer for...of / for...in variants as they are less error prone to off-by-ones for example, especially when just writing example code without extensive testing.
Relevant code guide sections:
"Update class syntax to use class fields (as in, replace
this.foo;withfoo;in class declarations)"
Recommendation: ❌ The code guide directly comments on this: Class methods should use this or be made into a static method. We should either follow the guide completely or look for another one. Although I agree that removing this from the examples would reduce redundancy and give our customers less to read, using var or leaving out the this keyword may lead to name clashes that can go unnoticed - so it's probably best to stick with the guide's recommendations.
Relevant code guide sections:
- clases & constructors
"Class methods should use this or be made into a static method unless an external library or framework requires using specific non-static methods. Being an instance method should indicate that it behaves differently based on properties of the receiver. eslint: class-methods-use-this" Quote from 9.7
"Use
staticmethods andblocksfor shared utilities specific to an example."
Recommendation: ✅ The code guide does not contain this topic - but in my opinion it's a good idea to use static init blocks (I'd call them static constructors, but that might just be me, as my background is in C++) so every variable is scoped to the class / type it belongs to and doesn't clutter the global namespace. In that regard it's similar to the var vs. let debate.
"Standardize on using a global object when creating new functions and classes. A lot of examples do this but some do not and it is not called out in the code guide. (For example, use const aria = aria || {};)"
Recommendation: I don't really have an opinion on this - I'd like to see some examples first. @outofambit can you share one or two maybe so I can see what exactly you would like to change?
PS: I hope this post is to your liking. As I'm still new to the group I tried to make sure to go into as much detail as seemed reasonable, but feel free to critique me 😉
The ARIA Authoring Practices (APG) Task Force just discussed Code guide updates.
The full IRC log of that discussion
<jugglinmike> Topic: Code guide updates<jugglinmike> OliverH: I reviewed the code guides that we referenced at our previous meeting
<jugglinmike> OliverH: I tried to match the assumptions in the thread with the content in those guides, and I gave recommendations for each
<jugglinmike> github: https://github.com/w3c/aria-practices/issues/3060
<jugglinmike> Matt_King: I think I would like to move the content of the guide guide out of the wiki because we can't submit pull requests against the wiki
<jugglinmike> Matt_King: We do have a place in the APG for information like this
<jongund> https://github.com/w3c/aria-practices/wiki/Code-Guide
<jugglinmike> Matt_King: If I created a "shell" page for this, would you be willing to draft a patch that relocates the content?
<jugglinmike> OliverH: Sure
<jugglinmike> OliverH: I'll plan to submit this with one commit that simply relocates the information and a separate commit that proposes changes so that it's easier to understand the modifications we're considering
<jugglinmike> Matt_King: That sounds great!