-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Update GitHub Skills format for accessibility #408
Conversation
README.md
Outdated
@@ -46,7 +49,7 @@ People use GitHub to build some of the most advanced technologies in the world. | |||
}).toString() | |||
--> | |||
|
|||
[![start-course](https://user-images.githubusercontent.com/1221423/235727646-4a590299-ffe5-480d-8cd5-8194ea184546.svg)](https://github.com/new?template_owner=skills&template_name=introduction-to-github&owner=%40me&name=skills-introduction-to-github&description=My+clone+repository&visibility=public) | |||
[![start-course](https://user-images.githubusercontent.com/1221423/235727646-4a590299-ffe5-480d-8cd5-8194ea184546.svg)](https://github.com/new?template_owner=skills-dev&template_name=introduction-to-github&owner=%40me&name=skills-introduction-to-github&description=My+clone+repository&visibility=public) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, temporary change, will revert this line before merge.
.github/workflows/0-welcome.yml
Outdated
|
||
# In README.md, switch step 0 for step 1. | ||
- name: Update to step 1 | ||
uses: skills-dev/action-update-step@main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update these to skills/action-update-step@v2
after update skills/action-update-step ships
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a content strategy/design perspective, this looks like a good way to present the lesson. I made a few small suggestions in the steps
, but no major changes.
I didn't review the files in workflows
, but I tested the course and it all worked for me 🎉
The branch will automatically switch to the one you have just created. | ||
The **main** branch drop-down bar will reflect your new branch and display the new branch name. | ||
|
||
6. Wait about 20 seconds then refresh this page (the one you're following instructions from). [GitHub Actions](https://docs.github.com/en/actions) will automatically update to the next step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would be helpful to send people to https://docs.github.com/en/actions/learn-github-actions/understanding-github-actions instead of the Actions landing page? It's a pretty dense article, but maybe fits the need for why someone would click the link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hehe the first sentence "GitHub Actions is a continuous integration and continuous delivery (CI/CD) platform" which isn't what Skills is using it for. More broadly its event hooks + compute 🤷🏼 https://docs.github.com/en/actions/using-workflows/about-workflows A little weird but the content is similar and lines up with Skills case more 🤷🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That article looks like a better fit, but I think it would also be fine to leave it as the landing page and let people explore.
No matter which article you go to, if you're brand new to GitHub and end up in the Actions docs you're probably going to 🤯
|
||
**What is a merge?**: A _[merge](https://docs.github.com/en/get-started/quickstart/github-glossary#merge)_ adds the changes in your pull request and branch into the `main` branch. For more information about merges, see "[Merging a pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request)." | ||
|
||
As noted in the previous step, you may have seen evidence of GitHub Actions running which automatically progresses your instructions to the next step. You'll have to wait for it to finish before you can merge your pull request. It will be ready when the merge pull request button is green. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when the merge pull request button is green
Since we're making these updates with accessibility in mind, is there a non-visual signal that a PR is ready to merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a throwback to how we used to say it during in-person classes (which is the origins of Learning Lab and now Skills). I'm a proponent of changing it for better accessibility!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open an internal issue for an accessibility content review of the courses, and link to this as an example.
|
||
1. Click **Merge pull request**. | ||
2. Click **Confirm merge**. | ||
3. Once your branch has been merged, you don't need it anymore. To delete this branch, click **Delete branch**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might happen automatically by default, but I'm not 100% sure if that's just how my settings are configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's configured per-repo. Although the template does have auto-delete of the branches turned on, the repo created for me (as the user) did not. I'm not sure if that can be configured for them.
2. Create a file named `README.md` in its root. The "root" means not inside any folder in your repository. | ||
3. Edit the contents of the `README.md` file. | ||
4. If you created a new branch for your file, open and merge a pull request on your branch. | ||
5. Lastly, we'd love to hear what you thought of this course [in our discussion board](https://github.com/skills/.github/discussions). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5. Lastly, we'd love to hear what you thought of this course [in our discussion board](https://github.com/skills/.github/discussions). | |
5. Lastly, we'd love to hear what you thought of this course [in our discussion board](https://github.com/orgs/skills/discussions/200). |
Maybe we would link specifically to this discussion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, I think it just adds to content maintenance overhead as we may eventually not want to use that discussion anymore. But I'm still a proponent of linking to the proper place - currently the discussions are unorganized and self-serve it seems?
If we do this I suggest creating an official discussion, or better yet a Category, and probably a conversation about who looks after these discussions (or maybe that already exists and I'm just not aware).
Edit: clarify what I meant about discussions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open a separate internal issue about discussion organization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking suggestion - especially now that we have multiple files, it might make it easier to also provide some error handling/troubleshooting. For example, I purposefully added a file to the wrong branch.
Edit: forgot to say, I tested the course and reviewed the workflows. Looks good to me!
Co-authored-by: Ethan Palm <[email protected]>
It's a good idea, I think starting with a test case in the Action repo would be good. Created: skills/action-update-step#12 |
Co-authored-by: Courtney Wilson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the work that went into this! I tested it, too, and it works great!
Co-authored-by: Courtney Wilson <[email protected]>
Co-authored-by: Courtney Wilson <[email protected]>
This comment was marked as spam.
This comment was marked as spam.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also followed the course without issue ✨
My only qualm is the wait between steps. I found myself hitting refresh a few times before the next step appeared. I imagine there's not much we can do here, but is there any way to indicate that the action is in progress? Otherwise, I think the "20 second wait" message at the end of each step is clear 👍
I have tested the @skills-dev course. |
amazing work |
I'll open an internal issue to investigate. |
In a recent accessibility audit, it was determined there was no way to use <details>/<summary> in an accessible way, and we didn't want to completely hide all step content from learners. Updates the course format to split out the steps into separate step files, replaces README content after completion of a step.
To test the course, please test the course directly at https://github.com/skills-dev/introduction-to-github
There's an order of operations hitch here with the update-step workflow, that is temporarily set to the skills-dev org for now. If we decide to move forward, we'll need to update skills/action-update-step first, then edit the fork to use skills/action-update-step@v2, then this will be in a mergeable state.