Update wins.js so the Wins page uses icon-github.svg

[izma-mujeeb]

Fixes #6911

What changes did you make?

• In the wins.js page, I replaced the image that was initially being used, with the image that used an svg tag. This image has a name of "icon-github.svg"

Why did you make the changes (we will use this info to test)?

• I had to change the image because using the original image was causing a redundancy in the number of images. By using an image with the svg tag, I am ensuring that there are no duplicate images.

Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)

• No visual changes made to the website

[github-actions]

Want to review this pull request? Take a look at this documentation for a step by step guide!

---

From your project repository, check out a new branch and test the changes.

git checkout -b izma-mujeeb-update-wins.js-to-include-svg gh-pages
git pull https://github.com/izma-mujeeb/website.git update-wins.js-to-include-svg

[codyyjxn]

Availability: M > F
ETA: EOD

[codyyjxn]

Hi @izma-mujeeb Thank you for taking on this issue!

• The branch name is correct but I recommend you include the issue number for future reference.
• The issue is linked correctly
• The changes are correct I checked on my end I was able to see the changes using docker.
• The title and description are accurate to understand what the task/change.

Great Job! Thanks for your contribution.

[8alpreet]

Availability: 3-7pm, Mon-Fri
ETA: EOD Tue, Oct 8

[8alpreet]

I am testing these changes locally and the icon is not displaying for me. Does it display for both of you @izma-mujeeb and @codyyjxn?

I'm still doing research, but all other instances of the _includes/svg/icon-github.svg use the include tag or are within a .yml file (which I don't know the internals of). I haven't found any instances of a .svg from _includes being used with an img tag. Jekyll does not treat files in the _includes directory as static assets, so we should not be able to use it as the value of the src attribute in an image.

[codyyjxn]

I am testing these changes locally and the icon is not displaying for me. Does it display for both of you @izma-mujeeb and @codyyjxn?

I'm still doing research, but all other instances of the _includes/svg/icon-github.svg use the include tag or are within a .yml file (which I don't know the internals of). I haven't found any instances of a .svg from _includes being used with an img tag. Jekyll does not treat files in the _includes directory as static assets, so we should not be able to use it as the value of the src attribute in an image.

Hey @8alpreet yea just check and for some reason I cant see the icon either. I think maybe the svg isn't rendering correctly.

[codyyjxn]

Hey @izma-mujeeb I was looking into the code and @8alpreet actually brought up a good point but we arent able to see the github icon. Please provide screenshots of what you are seeing on your end. The before and after you fix the code. Thank you ! If you have any question feel free to message any of us.

[k-cardon]

In case it's helpful--here's the error message that came up when I ran the docker container locally:

hfla_site | [2024-10-11 04:41:18] ERROR '/wins/_includes/svg/icon-github.svg' not found.

I also can't see the icon, so there's definitely a bug. Is wins/_includes the correct path?

[t-will-gillis]

Hey @izma-mujeeb @8alpreet @k-cardon @codyyjxn I took a stab at figuring this out also- I am getting no further than you are. The path _includes/svg/icon-github.svg appears correct.

I do not know Liquid, but I am thinking that maybe(?) Liquid wants these assets to be in the /assets/ folder. There is the footer.html that uses the same GitHub icon. I am not certain but to me it doesn't look like there is a simple 'swap-out-the-new-path' substitution.

@roslynwythe do you know more about this?

[FamousHero]

Hi @izma-mujeeb, thanks for taking on this issue!

In terms of pull request formatting, everything looks great! I do agree with @codyyjxn that the issue number should be included on the branch name but besides that there are no issues. Like others have said, I'm having issues seeing the changes on my machine. I'm unsure what the exact issue is but Jekyll has great documentation and if not mistaken should have a section on static assets and includes

Jekyll Docs

[roslynwythe]

@izma-mujeeb The liquid statement {% include svg/icon-github.svg %} will result in an inline svg, which should replace the existing img tag. To accomplish this, I think you will want to make changes to both wins.js and the html template (_includes/wins-page/wins-card-template.html/wins-card-template.html). I should have mentioned that in the issue. I added an Action Item and Resources to the issue, to clarify.

Part of the issue is making the svg accessible. The Resource making SVGs WCAG compliant provides sample code for doing that.

We could setup a Zoom meeting to go over this, or we could discuss it in the Thursday office hours. Please send me a Slack message and let me know your availability. If you would like to meet, please send me a Slack message.

[roslynwythe]

The issue has been edited to:

• reduce scope to replacing the GitHub icons in the wins cards, which appear on page load. When the wins card is expanded ("See More"), an overlay appears. Initially this issue was intended to address the wins card and the overlay, but a new issue ER: Update wins.js so the Wins overlay uses icon-github.svg #7727 has been created to address the overlay.
• include a reference with sample code for making the svg accessible
• include mention and references to _includes/wins-page/wins-card-template.html/wins-card-template.html