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

Revert "Updates to image pathing" #719

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

sinsukehlab
Copy link
Contributor

@sinsukehlab sinsukehlab commented Apr 30, 2024

Reverts #716

Summary

PR #716 has broken the image paths. This PR will revert #716.

Changes

Task list

  • For workflow changes, I have verified the Actions workflows function as expected.
  • For content changes, I have reviewed the style guide.

@ValkyrieSigrun

This comment was marked as spam.

@chadfawcett chadfawcett requested a review from a team April 30, 2024 19:32
@heiskr
Copy link
Contributor

heiskr commented Apr 30, 2024

Hmm yeah it does look broken to me as well on main currently, I don't have context on #716 either. Maybe we could use absolute URLs?

@hectorsector
Copy link

Thanks for the revert, @sinsukehlab. Merging this in - the relative references break the content for the majority of users.

@WirelessLife since you merged in #716, this change breaks the course so we're reverting it. We can chat about potential long term solutions for your use case if this way of referencing images isn't working.

@hectorsector hectorsector merged commit 86a4bea into skills:main Apr 30, 2024
@sinsukehlab
Copy link
Contributor Author

In Markdown files in a repository, the root (/) indicates the root directory of the repository on the current branch. Relative references will indicate different paths between when written in /.github/steps/*.md and when written in /README.md. You might want to use the parent directory of the root (/../) to refer to things other than the files, such as Actions workflow runs, Projects, Wiki, Security alerts, Insights, Settings, and Tags, Releases, and Packages.
In comments on Issues, PRs, and Discussions, the root (/) indicates the root of https://github.com.

@sinsukehlab
Copy link
Contributor Author

🙄
@WirelessLife @SkillBL It's okay to use this course as a part of hands-on labs, but when you have trouble with implementation, you should first try to resolve it in your platform. When you definitely believe that the course itself should be changed, you should check if the course functions as expected before you submit (for contributors) or merge (for maintainers) PRs.
All GitHub Skills courses excluding secure-code-game use the action-update-step action to update the step. When updating the step, README.md body is replaced with the corresponding Markdown file /.github/steps/*.md. Learners follow instructions from README.md. This is why relative references shouldn't be used in instructions.

So there are plans to use this Codespace as part of future hand-on labs. We are working with a company called Skillable who will be hosting the labs at the event. The issue with the broken image paths on Skillables platfform stemmed from how they were referenced in the project. Initially, Skillable had issues leveraging the absolute paths like /images/create-new-file.png, which worked fine when viewing the files directly on GitHub, but it caused issues when Skillable pulled the includes onto their platform.

When Skillable tried to include these images using relative paths like ../../images/create-new-file.png, they displayed properly on GitHub and within the Skillable client. I'm thinking this was because the relative path calculations were different depending on the location of the file including the image.

To fix this, the paths were updated to ../../images/create-new-file.png and submitted a pull request, which essentially navigated two directories up from the current location before locating the images directory. This ensured that regardless of where the file including the image was located within our project's directory structure, it would correctly resolve to the create-new-file.png image inside the images directory.

Seeing that this then broke another lab, I'm open to suggestions regarding next steps to ensure all labs are not affected. Thank you all for your help on this.

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.

5 participants