Modal Dialog Pattern: Clarify initial focus and aria-describedby guidance

[patrickhlauke]

• reworks/expands the initial focus section for modal dialogs to include the case of structured content dialogs (not necessarily covered by the "large enough to cause scrolling")
• adds advice about aria-describedby and when it's best NOT to use it (for structure-heavy dialogs)

Closes #442

---

Preview | Diff

[patrickhlauke]

happy for further wordsmithing/tweaking ... thought i'd just take a stab at this to start to try and address #442 which unfortunately still bubbles up as a topic of discussion in audits

[carmacleod]

Thanks, @patrickhlauke !

I do think that this clarifies, as well as adding the new point about structured content.
I like that "Generally, focus is initially set on the first focusable element." was moved outside of the list of examples, making it the default unless other conditions are met.

I just have a couple of suggestions: deleting some words, and using " in code examples instead of <q> (thanks for pointing that out 😄 ).

[patrickhlauke]

@carmacleod accepted your suggestions. thanks.

[patrickhlauke]

as an aside ... I see lots of approved pull requests in this repo, but they're lingering without being merged. intentional? do you merge them all in one big go, or...

[carmacleod]

as an aside ... I see lots of approved pull requests in this repo, but they're lingering without being merged. intentional?

@mcking65 does the final review and merging, and he's a busy guy. :)

Looking at open PRs and filtering Reviews by "Approved review", I see that there are 15.

Of the 8 that were created in December, most are either infrastructure or unit test related, so they could go in with one approving review. They're just stuck in the "Holiday Backlog". ;)

Of the 7 earlier PRs, 1 is infrastructure (but touches 77 files and needs to be rebased again), and several are either new or completely refactored examples (which require a bunch of reviewing/testing and at least 3 approving reviews). I'll look into the remaining few to see if I can move them along.

Of course, if you are offering to help review open PRs, that would be super awesome! 😄

[carmacleod]

+1

Thanks @patrickhlauke !

[mcking65]

@patrickhlauke wrote:

as an aside ... I see lots of approved pull requests in this repo, but they're lingering without being merged. intentional? do you merge them all in one big go, or...

The number and type of needed approving reviews depends on what the PR is changing. If a PR adds or significantly changes an example, we require all the reviews listed in our PR review process. As appropriate, we remove various reviews from the process.

We've been working at making a process that is both more disciplined and rigorous to address feedback on quality but at the same time find ways of improving efficiencies. It has been a really tricky balance given the number of active participants in the task force and the level of support resources we have available. I'm optomistic that in 2021 we will be able to make significant gains in both quality and efficiency through stronger community engagement programs.

[mcking65]

@patrickhlauke

Thank you very, very much for this PR!

I'd like to ensure the purpose of the guidance and when it applies is a bit more clear. Also, we can simplify some of the language for consistency with the rest of APG. I made two suggestions to these ends.

[patrickhlauke]

Accepted the suggested rewordings. thanks @mcking65

[mcking65]

@patrickhlauke

Thank you again for this PR. It provides meaningful improvement to the clarity of the guidance. And, I'm really happy to close an issue that has been outstanding for more than 2 years!

As this is a clarification of existing guidance as opposed to a change, I'm comfortable classifying this as editorial and merging given that it also as approval from @carmacleod.