<scribe> scribe: MarkMccarthy
<Jemma_> meeting info?
<Jemma_> https://github.com/w3c/aria-practices/wiki/Pull-Request-Review-Process
mck: one of the elements of
feedback we received had to do with repuation/quality of APG,
which we've been working on improving
... there are probably more bugs than we'd like though
<Jemma_> reviews we are thinking are editorial review, functonal reveiw, visual design review, accessibility review, code review, test review
Matt_King: ZoeBijl and Jemma_ and I talked about this yesterday; what the process should be; what will be needed on various PRs
<Jemma_> https://github.com/w3c/aria-practices/wiki/Pull-Request-Review-Process
Matt_King: we've broken things up
into 6 different aspects (which jemma listed above)
... we want to make sure all this is clear, and to make sure
each relevant review has been completed (and by whom) for a
given PR
... [going through review process link]
... editorial is stuff that applies to prose
... I want to take the temp of the group on touch - going
forward should we include or exclude from these reviews?
carmacleod: we should do it; at least we'd know if we can ignore it or put in a warning
jongund: +1
Matt_King: okay great. if we're doing touch, should we add to the browser list or is it still the same?
jamesn: same browsers, different platforms. and Windows touch
Matt_King: when we say that this
piece of work is done, what would we consider sufficient?
That's what I'm after
... we aren't necessarily creating the most robust reusable
library, we're demonstrating how to use ARIA. we haven't said
our examples necessarily work on mobile. would it be sufficient
to use iOS+Safari and Android+Chrome?
jamesn: might depend on the complexity of the example. for some, if it works in one it should work in all. others might be much more complex
Matt_King: so this is maybe like the editor's judgement?
jamesn: yeah; at least one touch browser should be on the list
carmacleod: +1
jamesn: if we make it comprehensive, everyone has to have at least one of those devices
Matt_King: [reading back through
interaction notes on review page]
... is this wording good? well enough?
carmacleod: i think so. it's hard to be more specific
Matt_King: okay, that's what i
was thinking
... visual design review - I think we've agreed on this but
want to capture it
... we don't envision having a style consistent across all
examples, right? we want flexibility in presentation,
right?
... for instance, if we're mimicing something (like date
picker)
... what ZoeBijl said yesterday is for it to not "feel out of
date"
carmacleod: good point
Matt_King: so, all said, is this sufficient?
carmacleod: I think so?
sarah_higley: should we include color contrast in there?
Matt_King: we have a separate accessibility review
sarah_higley: ah okay
Matt_King: there's another note
where if a particular implementation isn't terrible accessible,
we encourage people to make it better
... didn't know how specific to get for accessibility review,
so i included basics. mostly just said review for WCAG
compliance and we'd allow for some AT failures in very
particular cases
jongund: i think we should
include high contrast mode
... unless that's part of the WCAG requirements note, but maybe
there should be a list of expected configurations etc.
carmacleod: yeah
jamesn: i agree
... it's unfortunate that not everyone can test for it, it's
only on Windows
jongund: well we have a whole group, not everyone has to do everything, and we can build in some expertise/specialization for reviewers
Matt_King: right, we want to make
sure responsibilities are well distributed across the group.
well said jongund
... so i originally put "with special attention to:" and a
sublist. but if I leave some stuff out maybe people won't look
at the others?
jongund: maybe the sublist should be roles, especially if we're doing segmented testing
Matt_King: I could keep it simple in the overview and just put something like "and including testing WCAG in Windows HCM"
jongund: yes, but i think we
should have some page where we list what tests people should go
through (Whether individually or not) so each role is
clear
... so anyone developing a test knows they have to do A B C
etc.
<Jemma_> accessibility checklist!
jongund: "this is my role as HC tester, I have to do XYZ"
jamesn: should we also have these requirements around axe?
carmacleod: i was thinking the same
jamesn: for instance, if it's according to spec etc.
carmacleod: does axe have a way to do that?
jamesn: well, we can specify what kind of failures we'd allow etc.
carmacleod: so make a point that says "manually run axe" or something?
jamesn: yeah
carmacleod: gotcha, yeah.
jongund: i'm not sure what we'd get from that though, but people can do it if they want
<sarah_higley> why not run axe-core in the functional test suite, and block merging on failed tests? (with the possibility to exclude specific tests if they're known failures)
carmacleod: it does check for multiple IDs etc., little things that can be missed
jamesn: and it's a good test to
run
... but we can't not check things in if there are errors
Matt_King: right
... i did make a note to say including Windows HCM
sarah_higley: it does make sense to have have axe in there, and also *not* block things. but approving those exceptions etc. will help with preventing regression
jongund: what happens when axe
changes?
... are we setting up a maintainence nightmare?
sarah_higley: well not a
nightmare, just basic stuff. we can pin the version if that's a
concern or not auto updating major versions
... that's just part of the whole thing
<jamesn> +1
Matt_King: is axe in a node package we can maintain?
<Jemma_> -1 for specific tool dependency issue
sarah_higley: yes, i believe so
<Jemma_> +q
Matt_King: so we can make
adjustments really quick if we need to; okay sounds like a good
idea
... if someone can get a PR together that'd be good
ack Jemma_ m
Jemma_: should we specify the tools we're using? or ... what? I want to be cautious about hard specifications of tools. we can talk about this next time, no need to hold up today
<jongund> sorry, I need to go to another meeting
Matt_King: okay. i think people
are suggesting because axe is open source
... [reading through code review and test review]
... ZoeBijl is working on the code review guide, thanks for
helping everyone!
... test review, we have some pretty specific requirements, and
we have at least three people on that. valerie will also be on
that
<Jemma_> s/...what/specific version of the tool
sarah_higley: is there any interest on the automated tests we discussed before? for javascript etc.
Matt_King: can you restate? an expansion of the scope of our regression tests?
sarah_higley: basically. for instance there might be lots of logic behind how something happens, or other funky edge case behavior
Matt_King: i thought we did do
some of that testing in most of our tests. for instance the
wrapping with keyboard logic, we do test that
... particularly in grid
sarah_higley: i haven't looked at that specifically, but just wondering if there's a mapping between test IDs and other chunks
Matt_King: we can discuss further
to find out if there's gaps. there's some other work (like
globally checking we're not capturing keys we shouldn't
be)
... so yes, there's certainly validity in improving regression
tests
Jemma_: is there a space we can set up where sarah_higley can make these comments, as well as some other members working on this?
Matt_King: for comments on the review process, we can open another issue. if someone could open one that'd be great
Jemma_: evan, CurtBellew, any other suggestions?
evan: I generally agree with sarah_higley, there's lots of stuff that's basically not covered. we're only testing the ARIA stuff, so we'd be missing a lot
<CurtBellew> apparently my audio isn't working. sorry.
<CurtBellew> There was nothing I disagreed with
<sarah_higley> I can open an issue on unit/regression testing specifically
Jemma_: thanks sarah_higley
<Jemma_> https://github.com/w3c/aria-practices/pull/1120
Matt_King: in PR 1120, i modified
the top comment and put in a heading of "review
checklist"
... there are links to the different descriptions and some
placeholders
... i'm anticipating that as people review this PR they'd say
what aspects of the example they looked at; the editors could
check items off this list and add names to it as we work
through it
... then we'd know when it's ready for merge, or that's the
idea anyway [laughs]
... so looking for some feedback here. want to try to get
through this in the next week if possible
carmacleod: the only thing that
comes to mind is that in the past, a bunch of people may have
done a comprehensive review
... now with the checklist, one person does editorial, one does
functional, etc.
... it sort of feels like it won't get as full coverage --
Matt_King: i didn't mean to imply only one person does one thing, it can be multiple
carmacleod: comma separated, I see
Matt_King: it might indeed take
multiple people to cover some of these things, like test review
for interest
... i just want us all to agree we've all thoroughly covered
things
Jemma_: so let's give it a try!
carmacleod: the proof of concept PR!
Matt_King: pilot PR
[laughs]
... I'm very glad jongund brought up HCM. so if someone says
"yes I checkced HCM" or something... we could make a more
detailed checklist, but we should be careful not to spiral out
of control
Jemma_: this is good for teamwork and should be more approachable as a reviewer
Matt_King: true. okay, so what we
can do is each week we can check in on what's done what's not,
what revisions, etc.
... easier way to keep track of progress on the PR
Matt_King: we have everything
from this milestone under a different issue except the menu
example (which valerie needs to look at?)
... or did you look at it sarah_higley?
sarah_higley: yes
Matt_King: did it look ready?
<Jemma_> https://github.com/w3c/aria-practices/milestone/10
sarah_higley: i proposed a test change and i think jongund already put it in
Matt_King: for the group - if stuff has already landed and we're generally happy with it, should we go through the review? well, no, i don't think we should burden smaller ones.
sarah_higley: yeah
Matt_King: we don't have a separate item for the listbox example. sarah_higley you worked on that right?
sarah_higley: yeah there's a PR ready
Matt_King: okay, so let's get some reviews for that this week
<sarah_higley> grouped listbox PR link: https://github.com/w3c/aria-practices/pull/1191
Matt_King: so everything in the milestone is covered. we're 2 weeks out so I think we'll land everything
carmacleod: great!
Matt_King: nice work
<sarah_higley> awesome!
https://raw.githack.com/w3c/aria-practices/979-meter-pattern/aria-practices.html#meter
Matt_King: sarah_higley, can you walk through revisions?
sarah_higley: ZoeBijl and carmacleod reviewed the pattern and those changes are in the PR. right now I think all the comments on the pattern are addressed
Matt_King: okay, this is good.
for one's like this, i can add the checklist, but i think all
that will apply is editorial
... Jemma_, do you want to talk about the example?
https://raw.githack.com/w3c/aria-practices/issue979-add-meter-example/examples/meter/meter.html
Jemma_: I think i've addressed
everything left in comments, except the last thing about
document selectors
... other than that, should be up to date
... sarah_higley was talking about namespaces or something like
that
Matt_King: interesting, JAWS says this is a progressbar, but this *isn't*, right?
Jemma_: yep
carmacleod: right
Matt_King: I wonder if that's a
Chrome mapping bug or a JAWS bug... anyway!
... let's test our new process with these too
Jemma_: i need to clean up some conflicts, but yeah we can do that
sarah_higley: point of clarification - it's more common to share commits on a single PR here, but for some comments, i'd be happy to update something and commit. what's most welcome?
Matt_King: depends on the nature of the change. correcting an obvious mistake (typos etc.) on the branch itself usually doesn't ruffle anyone
jamesn: you can do suggested changes right?
Matt_King: yep
jamesn: that's easy, and then people can see what's done
Matt_King: for me, it's easier to push a commit than do the suggest change feature. but yes, either. (that's just an a11y thing in github)
sarah_higley: that all makes sense, thanks!
Matt_King: you can also communicate directly with anyone and ask. but if it's something where you think there might be discussion, probably better to comment.
jamesn: so when do we expect meter to land? time is running out
Matt_King: i think we can land it in the coming week
jamesn: okay, cool
Matt_King: pattern at least, Jemma_ you're on track to take care of the example?
Jemma_: yeah
Matt_King: i can definitely commit to the pattern, the example is just a matter of how quickly to get reviews done
jamesn: we don't *need* the example though i guess
Matt_King: but it's simple!
Jemma_: also probably about 90%
done, then just tests
... just needed a little more time to figure out how best to
incorporate sarah's suggestions
<Zakim> jamesn, you wanted to ask when we expect meter to land
Jemma_: evan, can you see sarah's comments? some feedback from you would be helpful
evan: sure thing
Jemma_: thanks both of you!
https://github.com/w3c/aria-practices/pull/1204
Matt_King: i just have one thing.
in all the other sections of the grid and table properties we
have example tables. for this one, i'm struggling to figure out
what the simplest possible example would be?
... we can do one that's just colindextext, like a spreadsheet.
that's one option
... but if people have suggestions or could help create, that'd
be awesome
[silence]
Matt_King: so in the preview
link, in grid and table properties, i added one bullet to this
list
... that being, how to communicate cell position
... added two rows to the table in the intro, and section
7.5
... i'm trying not to muddy anything that's already clear,
especially with the anticipated low usage of these
attributes
... okay, so would appreciate some feedback on the draft PR and
perhaps some example markup
Jemma_: sure
Matt_King: well alright then, if there's no immediate feedback, let's adjourn! thanks everyone
<CurtBellew> Bye all
This is scribe.perl Revision: 1.154 of Date: 2018/09/25 16:35:56 Check for newer version at http://dev.w3.org/cvsweb/~checkout~/2002/scribe/ Guessing input format: Irssi_ISO8601_Log_Text_Format (score 1.00) Succeeded: s/[missed]/axe/ Succeeded: s/mused/missed/ FAILED: s/...what/specific version of the tool/ Succeeded: s/please use issue xxx/we can open another issue. if someone could open one that'd be great/ Present: Matt_King MarkMccarthy jongund Jemma_ carmacleod evan Bryan_Garaventa CurtBellew sarah_higley Found Scribe: MarkMccarthy Inferring ScribeNick: MarkMccarthy Agenda: https://github.com/w3c/aria-practices/wiki/October-8%2C-2019-Meeting WARNING: No date found! Assuming today. (Hint: Specify the W3C IRC log URL, and the date will be determined from that.) Or specify the date like this: <dbooth> Date: 12 Sep 2002 People with action items: WARNING: Input appears to use implicit continuation lines. You may need the "-implicitContinuations" option. WARNING: IRC log location not specified! (You can ignore this warning if you do not want the generated minutes to contain a link to the original IRC log.)[End of scribe.perl diagnostic output]