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

Content tweaks for building website (part of issue 2418 task 11) #2537

Merged
merged 16 commits into from
Nov 22, 2022

Conversation

alflennik
Copy link
Contributor

@alflennik alflennik commented Nov 11, 2022

The PR includes the practical miscellaneous changes needed to use the new content organization as the starting point to build the APG website. Just one example of the type of changes needed is the fact that the first sentence of each pattern is used as its description and the checkbox pattern did not actually begin with a complete sentence. The broken links were found as part of the link checker effort which itself should have a PR soon.

See related PR w3c/wai-aria-practices#170 for more information.


WAI Preview Link failed to build on 'Update site files' step. (Last tried on Fri, 11 Nov 2022 03:39:17 GMT).

@alflennik
Copy link
Contributor Author

alflennik commented Nov 11, 2022

The preview link build failure appears to be caused by a Git Submodule caching issue in Netlify, I will investigate this with Howard and get in contact with Steve at WAI who administers the ARIA-Practices Netlify account.

@alflennik alflennik changed the title Content tweaks for building website (Part of Issue 2418 task 11) Content tweaks for building website (Part of issue 2418 task 11) Nov 11, 2022
@alflennik alflennik changed the title Content tweaks for building website (Part of issue 2418 task 11) Content tweaks for building website (part of issue 2418 task 11) Nov 11, 2022
@alflennik alflennik requested a review from mcking65 November 11, 2022 03:59
@mcking65
Copy link
Contributor

@alflennik, I will review this afternoon (pacific time). I will try to do as early as possible so you have the feedbac/merge before end of your day.

When reaching out to Steve, also reach out to @shawna-slh. I believe @michael-n-cooper told me that Steve was leaving at some point. So, there is a possibility he is not managing the netlify account. I might not be remembering correctly, and I don't recall timing. Just want to be sure you are able to get timely response.

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, Looks awesome! I made necessary editorial changes. I have a few questions:

  1. Curious why img directory is in repo root instead of in say /content/shared/? Seems like we should consider images as part of the content.
  2. Should the spec link fixing work if I view files locally in browser? It is not working for me.
  3. Should insertion of example warning be working if I view example files locally? It is not working for me.
  4. What do you think of renaming /content/index.html to /content/apg-home.html? I suggest this because we have another file named index.html under /content/index.
  5. In /content/index.html, Could you please delete the <div id="work" that starts on line 61. We are not using it, and we don't have plans to restore it. It is most likely similar information will eventually end up under /about, and there is no reason to keep it in the home page file.
    Feel free to merge this to move-examples branch whenever you are ready.

content/shared/js/app.js Show resolved Hide resolved
content/shared/js/app.js Show resolved Hide resolved
@shawna-slh
Copy link

Unfortunately, dependency updates broke many Netlify previews. Hopefully we can get those fixed soon.

@alflennik
Copy link
Contributor Author

@mcking65 The feedback has been addressed, but before I merge I want to give you a chance to take a second pass over the changes I've made since last time which include fixing some garbled text and making the advisements consistent with notes and warnings.

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.

Thank you @alflennik! Many excellent corrections. Great content quality boost! Just one forward looking question below in the CSS.

.warning > h4,
.advisement > h2,
.advisement > h3,
.advisement > h4 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider specifying h5 even though we don't have any ... yet? There are a couple of practices, like keyboard, where we get down to h4 for subtopics.

@alflennik alflennik merged commit 8a603a2 into move-examples Nov 22, 2022
@alflennik alflennik deleted the move-examples-build-fixes branch November 22, 2022 19:24
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.

3 participants