Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add landmark pattern page #2670

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Add landmark pattern page #2670

merged 3 commits into from
Jun 8, 2023

Conversation

mcking65
Copy link
Contributor

@mcking65 mcking65 commented Apr 10, 2023 •
edited by michael-n-cooper-bot
Loading

Resolves #2645 by adding a new landmarks pattern page.

Netlify preview of the landmarks pattern page

WAI Preview Link failed to build on 'Update site files' step. (Last tried on Tue, 16 May 2023 18:47:50 GMT).

This was referenced Apr 10, 2023
Closed
Closed
@mcking65 mcking65 linked an issue Apr 10, 2023 that may be closed by this pull request
Patterns page missing grid tile; Landmarks tile links to landmarks practices instead of landmarks pattern #2343
Closed
@charmarkk charmarkk self-requested a review April 11, 2023 18:51
@a11ydoer a11ydoer requested review from charmarkk and curtbellew and removed request for charmarkk April 11, 2023 18:51
@a11ydoer
Copy link
Contributor

a11ydoer commented Apr 11, 2023 •
edited
Loading

@ccanash please make changes in wai repo to include/update landmark pattern.

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2670: Add landmark pattern page by mcking65.

The full IRC log of that discussion <jugglinmike> subtopic: PR 2670: Add landmark pattern page by mcking65
<jugglinmike> github: https://github.com//pull/2670
<jugglinmike> Matt_King: This is strictly editorial. There's no design work or coding work to review
<jugglinmike> Matt_King: We just need a couple people to look at the content and make sure we're good
<jugglinmike> Matt_King: This is a page within the "patterns" section for landmarks
<jugglinmike> Jem: The WAI preview failed to build
<jugglinmike> Matt_King: That's strange; why didn't it rebuild when I fixed the syntax error?
<jugglinmike> Alex: I'll look into it
<jugglinmike> Andrea: I can review the pull request
<jugglinmike> MarkMcCarthy: I can also review
<jugglinmike> CurtBellew: I'll review, too
<jugglinmike> Alex: this failed because it requires changes to the WAI-ARIA-Practices repo
<jugglinmike> Alex: assign it to Carmen at Bocoup (ccanash on Bocoup), and she'll pass it to the right person

@alflennik
Copy link
Contributor

The build failed because the wai-aria-practices repo needs to be updated, what makes this change special is that landmarks are given special treatment in the APG - they are categorized as both a pattern and a practice.

@andreancardona andreancardona marked this pull request as draft April 18, 2023 18:23
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2670: Add landmark pattern page by mcking65.

The full IRC log of that discussion <jugglinmike> Subtopic: PR 2670: Add landmark pattern page by mcking65
<jugglinmike> github: https://github.com//pull/2670
<jugglinmike> Matt_King: arigilmore has reviewed
<jugglinmike> Matt_King: We don't have a preview, though. arigilmore were you looking at the code diff?
<jugglinmike> Matt_King: Alex_Flenniken told us it failed to build
<arigilmore> https://github.com//pull/2622#issuecomment-1445448609
<jugglinmike> Matt_King: There are other people listed for review, but since we don't have a preview, let's mark the pull request as a draft
<jugglinmike> Matt_King: Hopefully we'll be able to get that into a ready-to-review state, but we'll have to talk with Alex_Flenniken

@a11ydoer
Copy link
Contributor

@alflennik can you help us to see the Preview URL?

@ccanash
Copy link

ccanash commented Apr 27, 2023

Hi @a11ydoer we can tackle this, we will triage it next week and post an update

@ccanash
Copy link

ccanash commented May 2, 2023

Hi @a11ydoer we have triaged it and assigned it for work during the current 2 week sprint that started yesterday. This will be done before the end of next week.

@alflennik alflennik mentioned this pull request May 3, 2023
Merged
@alflennik
Copy link
Contributor

alflennik commented May 3, 2023 •
edited
Loading

@a11ydoer thanks for your patience, the preview is here: https://deploy-preview-220--aria-practices.netlify.app/aria/apg/patterns/landmarks/

Once this PR is merged, the wai-aria-practices PR adding support for it should be merged: w3c/wai-aria-practices#220.

@mcking65 mcking65 marked this pull request as ready for review May 16, 2023 07:00
@mcking65
Copy link
Contributor Author

@alflennik
the preview is not updating when I push commits

@mcking65 mcking65 marked this pull request as draft May 16, 2023 18:45
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Add landmark pattern page.

The full IRC log of that discussion <jugglinmike> subtopic: Add landmark pattern page
<jugglinmike> github: https://github.com//pull/2670
<jugglinmike> Matt_King: The preview build hasn't rebuilt, and I don't know how to trigger it. I wish I knew how to do that
<jugglinmike> Matt_King: We're waiting on review from Mark, Andrea, and Kurt. But that might have to wait until we figure out how to rebuild the preview
<jugglinmike> jamesn: It looks like the script exited with a non-zero exit code. I think the script isn't reporting its error codes correctly
<jugglinmike> jamesn: But despite this, the relevant commits are rendered with green "check mark" icons on the pull request page
<jugglinmike> Matt_King: I've converted the pull request as a draft while we work this out. When we get the preview updated, I'll remove the "draft" status

@jnurthen
Copy link
Member

The error preventing this from building is:

/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/transformPatternIndex.js:86
throw new Error(
^

Error: Cannot insert the landmarks pattern link because the correct alphabetical position has diverged from a known state.
at /home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/transformPatternIndex.js:86:21
at Array.map ()
at transformPatternIndex (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/transformPatternIndex.js:68:10)
at async recursivelyCopyAllContent (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/recursivelyCopyAllContent.js:21:27)
at async preBuild (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/pre-build.js:19:3)

@mcking65 mcking65 mentioned this pull request May 16, 2023
Closed
@mcking65
Copy link
Contributor Author

@alflennik

Please see #2702. This came up during the meeting. It is not directly related to this PR, but I hope might reduce the possibility of preview failures in the future.

@alflennik
Copy link
Contributor

@mcking65 The aria-practices submodule for the PR on wai-aria-practices needs to be updated for the new commits to show. I'll be able to do that on Monday ... hopefully in the meantime the raw html preview is enough to get by. Moving forward sourcing this from aria-practices seems like a reasonable approach which would simplify the build process.

@alflennik
Copy link
Contributor

@mcking65 so sorry for the delay, the updated preview is here: https://deploy-preview-220--aria-practices.netlify.app/aria/apg/patterns/landmarks/

@mcking65 mcking65 requested a review from shirsha May 30, 2023 18:31
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Add landmark pattern page.

The full IRC log of that discussion <jugglinmike> subtopic: Add landmark pattern page
<jugglinmike> github: https://github.com//pull/2670
<jugglinmike> Matt_King: Alex_Flenniken has unblocked this, and we're waiting for reviews
<jugglinmike> Matt_King: None of the people assigned are in this call today. I can reach out to them via e-mail
<jongund> https://deploy-preview-220--aria-practices.netlify.app/aria/apg/patterns/landmarks/
<jugglinmike> siri: I will take a look

@mcking65 mcking65 marked this pull request as ready for review June 6, 2023 07:15
@ccanash
Copy link

ccanash commented Jun 8, 2023

Hi @charmarkk @curtbellew , we would like to include this in the Tuesday release next week. Would either of you have time to review it? Thank you

@mcking65
Copy link
Contributor Author

mcking65 commented Jun 8, 2023

thank you @curtbellew!

@mcking65 mcking65 merged commit fec7032 into main Jun 8, 2023
@mcking65 mcking65 deleted the landmarks-pattern branch June 8, 2023 16:49
@mcking65 mcking65 added enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy documentation Pattern Page Related to a page documenting a Pattern labels Jun 8, 2023
@mcking65 mcking65 added this to the H1/2023 Content Updates milestone Jun 8, 2023
@howard-e howard-e mentioned this pull request Jun 13, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@curtbellew curtbellew curtbellew approved these changes

@andreancardona andreancardona andreancardona approved these changes

@shirsha shirsha shirsha approved these changes

@charmarkk charmarkk Awaiting requested review from charmarkk

Assignees

@alflennik alflennik

@ccanash ccanash

Labels
enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Pattern Page Related to a page documenting a Pattern
Projects
Landmark Regions Guidance and Examples Development Project
Status: Complete
Milestone
2023-06-20 Publication Pull Requests
9 participants
@mcking65 @a11ydoer @css-meeting-bot @alflennik @ccanash @jnurthen @shirsha @andreancardona @curtbellew