Skip to content

Conversation

@santi-jose
Copy link
Member

@santi-jose santi-jose commented Oct 22, 2025

Fixes #6911

What changes did you make?

  • Within HTML template _includes/wins-page/wins-card-template.html/wins-card-template.html, replaced current reference to github-icon using the img element, with new inline expression rendering github-icon within a div element.
    Specifically replaced:
    <a target="_blank" class="wins-card-github-icon"><img class="github-icon" alt="" /></a>
    with:
<a target="_blank" class="wins-card-github-icon" style = "color:black">
  <div role = "img" class = "github-icon">
      {% include svg/icon-github.svg %}  
  </div>
</a>
  • Within the makeCards function in assets/js/wins.js, removed the img element source definition since we replaced the img element.
~~cloneCardTemplate.querySelector('.github-icon').src = GITHUB_ICON ;~~
  • Removed the definition of the constant variable GITHUB_ICON which set the path to the svg used to define the source for the previously used img element.
~~const GITHUB_ICON = '/assets/images/wins-page/icon-github-small.svg';~~
  • Made SVG WCAG compliant. Added aria-label attribute and definition to the github-icon's svg div. Used the previous definition of the alt attribute of the img element for the aria-label of the github-icon svg div.
cloneCardTemplate.querySelector('.github-icon').setAttribute('aria-label' ,`GitHub profile for ${card[name]}`);
  • Updated _sass/components/_wins-page.scss to ensure the styling of the GitHub icon remained unchanged after replacing the source. Set the github-icon element in _wins-page.scss height and width to 32px. Also set themax-width to 100% to mimic the behavior of the LinkedIn icon. Set the SVG width and height properties nested within the github-icon element divto 100% so they would adhere to the sizing of the parent element. In this case the github-icon div is the parent element. Therefore the SVG should render to be 32x32 px.
.github-icon{
  width: 32px;
  height: 32px;
  max-width: 100%;
}

.github-icon svg{
  width: 100%;
  height: 100%;
}

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

  • To reduce the number of duplicate images in the codebase.

CodeQL Alerts

After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.

Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown

Screenshot 2024-10-28 154514

Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.

  • I have checked this PR for CodeQL alerts and none were found.
  • I found CodeQL alert(s), and (select one):
    • I have resolved the CodeQL alert(s) as noted
    • I believe the CodeQL alert(s) is a false positive (Merge Team will evaluate)
    • I have followed the Instructions below, but I am still stuck (Merge Team will evaluate)
Instructions for resolving CodeQL alerts

If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.

In general, CodeQL alerts should be resolved prior to PR reviews and merging

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

  • No visual changes to the website

…to the github icon saved within assets: /asstes/images/wins-page/icon-github-small.svg. Also Removed the literal definition of GITHUB_ICON where the path to this icon svg which is being replaced. Modified line where the alt property was defined for the img which used to store the svg. To comply with WCAG, we instead add the description to the aria-label using the setAttribute method.
…ppearance of linkedin-icon class elements. Because the github-icon svg is rendered inline, the viewBox inherits a larger sizing from the source file than the Linkedin icon. Inspecting the linked-in image revealed a base size of 32px x 32px. To replicate this sizing, we set github-icon div width and height to 32px by 32px and made a rule for the svg to fill the container width and height by 100%.
@github-actions
Copy link

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 santi-jose-update-wins_js-to-use-icon-github_svg-6911 gh-pages
git pull https://github.com/santi-jose/website.git update-wins_js-to-use-icon-github_svg-6911

@github-actions github-actions bot added role: front end Tasks for front end developers Complexity: Medium P-Feature: Wins Page https://www.hackforla.org/wins/ Feature: Refactor HTML size: 1pt Can be done in 4-6 hours labels Oct 22, 2025
@lc1715 lc1715 self-requested a review October 22, 2025 22:13
@lc1715
Copy link
Member

lc1715 commented Oct 22, 2025

Review ETA: 10/22/25
Availability: 1-5pm M-F

Copy link
Member

@lc1715 lc1715 left a comment

Choose a reason for hiding this comment

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

@santi-jose Great job working on this issue!

  • The PR contains the correct branch and linked issue
  • The code that renders the Github icon has been updated and the styling remains unchanged
  • There were no CodeQL Alerts detected

@santi-jose
Copy link
Member Author

Thanks for the review!🙏 @lc1715

@lc1715
Copy link
Member

lc1715 commented Oct 24, 2025

Sure, Thanks for working on this issue! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complexity: Medium Feature: Refactor HTML P-Feature: Wins Page https://www.hackforla.org/wins/ role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours

Projects

Status: PR Needs review

Development

Successfully merging this pull request may close these issues.

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

2 participants