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

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

Open
7 tasks done
Tracked by #7664
roslynwythe opened this issue May 29, 2024 · 25 comments · May be fixed by #7569
Open
7 tasks done
Tracked by #7664

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

roslynwythe opened this issue May 29, 2024 · 25 comments · May be fixed by #7569
Assignees
Labels
Complexity: Medium Feature: Refactor HTML P-Feature: Wins Page https://www.hackforla.org/wins/ ready for dev lead Issues that tech leads or merge team members need to follow up on role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours Status: Help Wanted Internal assistance is required to make progress

Comments

@roslynwythe
Copy link
Member

roslynwythe commented May 29, 2024

Overview

We need to update the code that renders the GitHub icon on the Wins page, to display _includes/svg/icon-github.svg instead of /assets/images/wins-page/icon-github-small.svg, in order to reduce the number of duplicate images.

Action Items

  • Update the makeCards function in assets/js/wins.js and the HTML template in _includes/wins-page/wins-card-template.html/wins-card-template.html so that the reference to /assets/images/wins-page/icon-github-small.svg in the wins cards are changed to _includes/svg/icon-github.svg. The icon will be displayed using an svg html element instead of an img element.
  • This issue does not include replacing the references to /assets/images/wins-page/icon-github-small.svg in the overlay (in the function updateOverlay)
  • Note that svg tags do not support an alt attribute for accessibility. Read making SVGs WCAG compliant to understand how an SVG can be made accessible. Select a method described in the section "Use an ARIA label in the div HTML tag wrapping the SVG" or the section "Use an outer SVG with aria-label or aria-labelledby". Again this will require updates to assets/js/wins.jsand_includes/wins-page/wins-card-template.html/wins-card-template.html'
  • Read Create wiki page: how to change the fill color of an svg #6654 to learn how to style svgs
  • In this issue we will not modify the GitHub icon in the badge section which is /assets/images/wins-page/wins-badges/github.svg
  • Remove the file currently used graphic file /assets/images/wins-page/icon-github-small.svg
  • Use Docker to preview the Wins page (/wins) to ensure that the appearance and behavior of the Wins page is unchanged both in mobile and desktop views.

Resources/Instructions

@roslynwythe roslynwythe added role: front end Tasks for front end developers Complexity: Medium P-Feature: Wins Page https://www.hackforla.org/wins/ Feature: Refactor HTML Draft Issue is still in the process of being created labels May 29, 2024

This comment has been minimized.

@roslynwythe roslynwythe changed the title Update wins.js so the Wins page uses icon-github.svg Update wins.js so the Wins page uses icon-github.svg May 29, 2024
@roslynwythe roslynwythe added Ready for Prioritization size: 1pt Can be done in 4-6 hours and removed Draft Issue is still in the process of being created size: missing labels May 29, 2024
@ExperimentsInHonesty ExperimentsInHonesty added this to the x. Technical debt milestone Jun 2, 2024
@tony1ee tony1ee self-assigned this Jun 8, 2024

This comment has been minimized.

@tony1ee

This comment was marked as outdated.

@ExperimentsInHonesty ExperimentsInHonesty moved this to In progress (actively working) in P: HfLA Website: Project Board Jun 23, 2024
@github-actions github-actions bot added the 2 weeks inactive An issue that has not been updated by an assignee for two weeks label Jun 28, 2024

This comment has been minimized.

@tony1ee

This comment was marked as outdated.

@tony1ee tony1ee added Status: Help Wanted Internal assistance is required to make progress and removed 2 weeks inactive An issue that has not been updated by an assignee for two weeks labels Jun 29, 2024
@tony1ee tony1ee moved this from In progress (actively working) to Questions / In Review in P: HfLA Website: Project Board Jun 29, 2024
@t-will-gillis t-will-gillis added the ready for merge team needs a senior review either to do some re writing or to approve it for ready for prioritization label Jun 30, 2024
@roslynwythe

This comment was marked as outdated.

@roslynwythe roslynwythe removed Status: Help Wanted Internal assistance is required to make progress ready for merge team needs a senior review either to do some re writing or to approve it for ready for prioritization labels Jul 1, 2024
@HackforLABot

This comment has been minimized.

@HackforLABot HackforLABot added the ready for dev lead Issues that tech leads or merge team members need to follow up on label Sep 14, 2024
@HackforLABot HackforLABot moved this from In progress (actively working) to New Issue Approval in P: HfLA Website: Project Board Sep 14, 2024
@terrencejihoonjung

This comment was marked as outdated.

@t-will-gillis t-will-gillis added Ready for Prioritization and removed ready for dev lead Issues that tech leads or merge team members need to follow up on labels Sep 19, 2024
@izma-mujeeb izma-mujeeb self-assigned this Oct 1, 2024
@izma-mujeeb izma-mujeeb moved this to In progress (actively working) in P: HfLA Website: Project Board Oct 1, 2024
@HackforLABot
Copy link
Contributor

Hi @izma-mujeeb, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:-
i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?)
ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

@izma-mujeeb
Copy link
Member

izma-mujeeb commented Oct 1, 2024

Availability: Mon-Fri 5PM - 9PM
ETA: Wednesday, October 2

@izma-mujeeb
Copy link
Member

I have a blocker.
I am trying to change the icon to include the svg file, however, every time I change the file, I get this error:
ERROR '/wins/_includes/svg/icon-github.svg' not found.
I am not sure why this error is coming up as the above is not a path that I am including in the JavaScript file. As well, since the svg file is under the _includes folder, I don't think these images can be used with an img tag, as these files are treated as static assets.

@izma-mujeeb izma-mujeeb moved this from In progress (actively working) to Questions / In Review in P: HfLA Website: Project Board Oct 16, 2024
@izma-mujeeb izma-mujeeb added the Status: Help Wanted Internal assistance is required to make progress label Oct 16, 2024
@DrAcula27
Copy link
Member

DrAcula27 commented Oct 30, 2024

@izma-mujeeb @t-will-gillis
I have tried prepending the file path, _includes/svg/icon-github.svg, with each of: /wins/, /, and ../../ and none resolved the issue. All still give the same error message:

ERROR `/wins/_includes/svg/icon-github.svg' not found.

Update

After prepending the file path with ../../ and using tab completion to finish the path, I get a new error:

`/_includes/svg/icon-github.svg' not found.

@izma-mujeeb
Copy link
Member

At the office hour today, we looked through other files that use svg images in a JavaScript file, and tested to see if the following URL would work: {% include svg/icon-github.svg %}. Unfortunately, this link did not work and the Github icon is not being displayed on the Wins page.

@ExperimentsInHonesty
Copy link
Member

ExperimentsInHonesty commented Nov 1, 2024

These are the svgs that are in _includes folders
https://github.com/hackforla/website/tree/gh-pages/_includes/svg

Here is the search I used

Here is the JavaScript file that has references to svgs that are in the _includes folder
https://github.com/hackforla/website/blob/gh-pages/assets/js/hamburger-nav.js

//Retrieve svg string from _includes
const burgerImage = `{% include svg/icon-hamburger-nav.svg %}`;
const burgerImageX = `{% include svg/icon-hamburger-nav-x.svg %}`;

function swapIcons(icon_id){
//Create dictionary for swap
const hamburger = {
'hamburger-nav': {"el":burgerImage,'opposite':burgerImageX},
'hamburger-nav-x':{'el':burgerImageX,'opposite':burgerImage}
}

@roslynwythe
Copy link
Member Author

roslynwythe commented Nov 3, 2024

At the office hour today, we looked through other files that use svg images in a JavaScript file, and tested to see if the following URL would work: {% include svg/icon-github.svg %}. Unfortunately, this link did not work and the Github icon is not being displayed on the Wins page.

  • @izma-mujeeb I believe that {% include svg/icon-github.svg %} is the correct liquid statement to reference the svg. If it is not working, I'll need to see the surrounding js and html code. Previously the HTML was using an img tag and that will have to be changed. For the purposes of styling and accessibility, we will probably want to wrap the svg in an outer svg. You can find sample code for that in Update wiki page with another method for making SVGs WCAG compliant #6653 (comment) which is listed as a resource item in this issue. If you wish, you can create a Draft PR so that I can see your code.

@ExperimentsInHonesty
Copy link
Member

ExperimentsInHonesty commented Nov 5, 2024

@izma-mujeeb please submit a draft pr. You submit a PR as normal and then change it to draft by clicking on the link that looks like this, under the assignees. Once you done that, stick this issue back into the questions column with a ready for dev lead label

image

@ExperimentsInHonesty ExperimentsInHonesty moved this from Questions / In Review to Prioritized backlog in P: HfLA Website: Project Board Nov 5, 2024
@ExperimentsInHonesty ExperimentsInHonesty moved this from Prioritized backlog to In progress (actively working) in P: HfLA Website: Project Board Nov 5, 2024
@izma-mujeeb
Copy link
Member

izma-mujeeb commented Nov 5, 2024

@ExperimentsInHonesty @roslynwythe I submitted a PR and have moved this issue back into questions column with the ready for dev lead label.
#7569

@izma-mujeeb izma-mujeeb added the ready for dev lead Issues that tech leads or merge team members need to follow up on label Nov 5, 2024
@izma-mujeeb izma-mujeeb moved this from In progress (actively working) to Questions / In Review in P: HfLA Website: Project Board Nov 5, 2024
@ExperimentsInHonesty
Copy link
Member

@roslynwythe spoke with @izma-mujeeb and revised the issue to make it clearer and reduce scope. See details in PR

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/ ready for dev lead Issues that tech leads or merge team members need to follow up on role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours Status: Help Wanted Internal assistance is required to make progress
Projects
Status: Questions / In Review
Development

Successfully merging a pull request may close this issue.

9 participants