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.

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

Source patterns and practices pages in APG (Issue #2702) #2869

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

alflennik
Copy link
Contributor

@alflennik alflennik commented Nov 30, 2023

Sources the patterns and practices pages and all content on them, including images, from the aria-practices repo instead of bolted on later within the wai-aria-practices repo.

The change can be previewed here: https://deploy-preview-280--aria-practices.netlify.app/

Since this PR includes changes to the structure of the content the automatic preview link generated below is not expected to work.

See #2702 for more information.

This PR should be merged before w3c/wai-aria-practices#280.


WAI Preview Link failed to build on 'Update site files' step. (Last tried on Thu, 30 Nov 2023 16:14:25 GMT).

@mcking65
Copy link
Contributor

mcking65 commented Dec 5, 2023

@alflennik

I'm not able to review because the files tab won't load. When there are a lot of changed files, there is something that makes Chrome and any screen reader freeze. I need to find another way to review.

However, I was expecting this pull request would only change 2 html files and possibly add some images. So, I am surprised to see 124 changed files. The only html files that I thought would be touched are patterns.html and practices.html. What are the other files changed by this PR?

Also, the preview is failing.

@mcking65
Copy link
Contributor

mcking65 commented Dec 5, 2023

@alflennik

OK, as is often the case, there is far more to our content than I realized. From the cmd line, I got the list of changed files. I see 31 changes are the addition of image files. I looked at diffs for a few of the HTML files locally, and see there is far more use of images than I knew about.

It's a happy thing to learn about all the work people have done to make a good visual presentation. I'm also now even more glad that we are moving control of that visual presentation into the content repo. Everything looks on track here, but it will take a bit of time for me to review.

It is important we get the preview working again. We are going to need others to review the preview and ensure the visual presentation is consistent with production.

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Finish repo restructure.

The full IRC log of that discussion <jugglinmike> Topic: Finish repo restructure
<jugglinmike> github: https://github.com//pull/2869
<jugglinmike> Matt_King: This is another one of those changes where it's focused on the back-end. We don't want the site to regress at all
<jugglinmike> Matt_King: We want people to compare every page of the website in the preview to the corresponding page in production
<Jem> https://github.com//issues/2702
<jugglinmike> Matt_King: That's over 100 pages, so we're going to have to split up the work
<jugglinmike> Matt_King: For more context: in the current architecture, almost all of the images are inserted via scripts during the build process
<jugglinmike> Matt_King: But the images are content, so we want to control them from the "content" repository. We don't want the build process to insert content
<jugglinmike> Matt_King: This build moves all of that content into the content repository (e.g. 31 SVG files). It changes every page that has at least one image--it adds the reference to the image as a relative link
<jugglinmike> Matt_King: Once this lands, we'll have full control over the content from the "content" repository
<jugglinmike> Matt_King: We've done this in phases and steps since the launch of the redesign last year. This is the final step in that long process
<jugglinmike> Matt_King: In the past when we've had to make large changes like this, we've divided up patterns alphabetically
<jugglinmike> Matt_King: It appears that the preview for this isn't working right now. Hopefully the preview will be fixed later today, so that if we assign reviewers during this meeting today, then those preview links will be available tomorrow
<jugglinmike> Matt_King: It might be a lot to ask to have this all reviewed by Monday, but I think the work will go pretty quickly
<jugglinmike> Matt_King: Here's what I have in mind for the reviewers we designate for this work: Go to the production site, load the page, then on the preview, go to that same page in another tab. If they look the same at first glance, then move on to the next page.
<jugglinmike> Matt_King: The goal is no differences. If there is a difference, we'd want to know about that and flag it in the pull request
<jugglinmike> Matt_King: If we can get this done by Monday, then we could get it deployed this year. Otherwise, we'll have to wait until next year because this change is too complicated for us to give to Shawn by the only other publication date this year (December 19)
<jugglinmike> howard-e: Fixing the preview will require assistance from W3C, but it's not clear who is able to help and if they are available today
<jugglinmike> Matt_King: I don't think we need to review the "About" page. Between the "patterns" pages and the "practices" pages, I think there are about 80 pages to review in total
<jugglinmike> Matt_King: jongund_ can you look at the "patterns" page and then the "patterns" page up through just before "radio group"?
<jugglinmike> jongund_: Sure
<jugglinmike> Ariella_Gilmore: I can take "radio" through "window splitter"
<jugglinmike> Jem: I can review the "practices" pages
<jugglinmike> Matt_King: Then we would have everything covered amongst the three of you
<jugglinmike> Matt_King: I don't think there are any images on the index page
<jugglinmike> Jem: And there are no images on the "about" page, either
<jugglinmike> Matt_King: Okay, then I think between jongund_, Ariella_Gilmore, and Jem, I think that covers it!

@alflennik
Copy link
Contributor Author

@mcking65 Sorry I forgot to add the preview link! I added it to the PR description. I should have remembered to do that when I opened the PR, it would seem I'm still getting in the swing of things after a while away from this repo.

Also, thanks for the kind comments!

Copy link
Contributor

@ariellalgilmore ariellalgilmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

radio through window splitter looks good too me!!

@jongund jongund self-requested a review December 7, 2023 16:32
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed Accordion through meter patterns and the patterns page, I didn't find any problems with images or content.

@mcking65
Copy link
Contributor

mcking65 commented Dec 8, 2023

Thank you, thank you @jongund and @ariellalgilmore!!!

@a11ydoer
Copy link
Contributor

a11ydoer commented Dec 9, 2023

@mcking65 I reviewed and compared the practice page with the PR version. I didnot find any discrepancies.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alflennik

Looking at the HTML, this looks really good to me. It fully resolves the issue. Based on the feedback from @jongund, @a11ydoer, and @ariellalgilmore, it seems that the presentation also remains as expected.

Thank you!!!

If @howard-e also agrees this is ready to merge, I will mergeMonday morning so we can get this into production this week.

Copy link
Contributor

@howard-e howard-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Shared my actual PR feedback in the build repo.

@howard-e
Copy link
Contributor

If @howard-e also agrees this is ready to merge, I will mergeMonday morning so we can get this into production this week.

@mcking65 this should be ready to merge when ready. Reminder of @alflennik's note in the top comment on the reliance of w3c/wai-aria-practices#280:

This PR should be merged before w3c/wai-aria-practices#280.

That PR has also been approved.

@mcking65 mcking65 merged commit 4bb1946 into main Dec 11, 2023
14 checks passed
@mcking65 mcking65 deleted the patterns-practices-pages branch December 11, 2023 18:02
@alflennik
Copy link
Contributor Author

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants