skills / introduction-to-github

Get started using GitHub in less than an hour.
MIT License
4.3k stars 1.85k forks source link

Revert "Updates to image pathing" #719

Closed sinsukehlab closed 4 months ago

sinsukehlab commented 4 months ago

Reverts #716

Summary

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

Changes

Task list

ValkyrieSigrun commented 4 months ago

i never sent this Account Owner J McNeil

On Tue, Apr 30, 2024 at 1:10 AM, adnangujjar98 @.***(mailto:On Tue, Apr 30, 2024 at 1:10 AM, adnangujjar98 < wrote:

@adnangujjar98 approved this pull request.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

heiskr commented 4 months ago

Hmm yeah it does look broken to me as well on main currently, I don't have context on https://github.com/skills/introduction-to-github/pull/716 either. Maybe we could use absolute URLs?

hectorsector commented 4 months ago

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

@WirelessLife since you merged in https://github.com/skills/introduction-to-github/pull/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.

sinsukehlab commented 4 months ago

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 commented 4 months ago

When written in /.github/steps/*.md (https://github.com/skills/introduction-to-github/tree/main/.github/steps/*.md): ../: /.github (https://github.com/skills/introduction-to-github/tree/main/.github/) ../../: / (https://github.com/skills/introduction-to-github/tree/main/) ../../images/: /images/ (https://github.com/skills/introduction-to-github/tree/main/images/)

When written in /README.md (https://github.com/skills/introduction-to-github/blob/main/README.md or https://github.com/skills/introduction-to-github/tree/main/README.md): ../: https://github.com/skills/introduction-to-github.com/blob or https://github.com/skills/introduction-to-github/tree ../../: https://github.com/skills/introduction-to-github/ ../../images/: https://github.com/skills/introduction-to-github/images/

sinsukehlab commented 4 months ago

:roll_eyes: @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.