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

Update GitHub Skills format for accessibility #408

Merged
merged 12 commits into from
May 30, 2023
Merged

Update GitHub Skills format for accessibility #408

merged 12 commits into from
May 30, 2023

Conversation

heiskr
Copy link
Contributor

@heiskr heiskr commented May 25, 2023

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.

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)
Copy link
Contributor Author

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.


# In README.md, switch step 0 for step 1.
- name: Update to step 1
uses: skills-dev/action-update-step@main
Copy link
Contributor Author

@heiskr heiskr May 25, 2023

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

Copy link
Contributor

@ethanpalm ethanpalm left a 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 🎉

.github/steps/3-open-a-pull-request.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

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.

Copy link
Contributor Author

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 🤷🏼

Copy link
Contributor

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.
Copy link
Contributor

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?

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!

Copy link
Contributor Author

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**.
Copy link
Contributor

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.

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).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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

Copy link

@hectorsector hectorsector May 25, 2023

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.

Copy link
Contributor Author

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.

Copy link

@hectorsector hectorsector left a 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 <56270045+ethanpalm@users.noreply.github.com>
@heiskr
Copy link
Contributor Author

heiskr commented May 25, 2023

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.

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 <77312589+cmwilson21@users.noreply.github.com>
Copy link
Contributor

@cmwilson21 cmwilson21 left a 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!

heiskr and others added 3 commits May 25, 2023 16:57
Co-authored-by: Courtney Wilson <77312589+cmwilson21@users.noreply.github.com>
Co-authored-by: Courtney Wilson <77312589+cmwilson21@users.noreply.github.com>
@hkl615

This comment was marked as spam.

Copy link

@joeoak joeoak left a 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 👍

joeoak

This comment was marked as duplicate.

@sinsukehlab
Copy link
Contributor

sinsukehlab commented May 29, 2023

I have tested the @skills-dev course.
It works pretty well, but Step 0 workflow runs twice on the initial push to main with one success and one error.
Similar issue: skills/connect-the-dots#32

@Gideonkipyegon
Copy link

amazing work

@heiskr
Copy link
Contributor Author

heiskr commented May 30, 2023

Step 0 workflow runs twice on the initial push to main with

I'll open an internal issue to investigate.

@heiskr heiskr merged commit bc2fa49 into skills:main May 30, 2023
@Gideonkipyegon

This comment was marked as spam.

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.

8 participants