Update wins.js so the Wins page uses icon-github.svg
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
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
Availability: M > F ETA: EOD
Availability: 3-7pm, Mon-Fri ETA: EOD Tue, Oct 8
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.
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.svguse theincludetag or are within a .yml file (which I don't know the internals of). I haven't found any instances of a .svg from_includesbeing used with animgtag. Jekyll does not treat files in the_includesdirectory 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.
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?
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 #7727 has been created to address the overlay.
- include a reference with sample code for making the
svgaccessible - include mention and references to
_includes/wins-page/wins-card-template.html/wins-card-template.html
@izma-mujeeb could you resolve the conflicts in assets/js/wins.js?
Review ETA: 12 PM 12/10/24 Availability: 9-3 EST mon-thurs
@k-cardon Thank you for your patience. Please let me know if it works now.
Great, thanks for making those changes! I confirmed that the GitHub logo svg is displaying correctly! Looks like everything is fixed now.
Hi @roslynwythe, @8alpreet, @FamousHero, @codyyjxn - @izma-mujeeb made changes and resolved merge conflicts and the PR is ready for your re-review. Please provide ETA for your review or if you cannot re-review, please comment and remove yourself from the Reviewers section. Thanks!
Review ETA: 11 Feb 2025 @ 8 PM (GMT) Availability: Daily @ 6 PM - 8 PM (GMT)
Availability: Weekdays 5:30pm - 10pm PST ETA: EOD March 23
Hi @izma-mujeeb - Could you let us know if you're still planning to work on this? If so, please share your ETA for completion. If we don't hear back, we'll unassign the issue so someone else can pick it up. Thanks!
Closing as original dev is inactive, the original issue will be reassigned.