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

Declare Loop Variable in primaryOnDarkHandler Function #5940

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

jphamtv
Copy link
Member

@jphamtv jphamtv commented Nov 28, 2023

Fixes #5706

What changes did you make?

  • Declared the loop variable in primaryOnDarkHandler on line 14 in the assets/js/design-system.js file

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

  • To resolve the CodeQL alert

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied Screenshot 2023-11-27 at 22 52 46
Visuals after changes are applied Screenshot 2023-11-27 at 22 52 59

Before and after screenshots from the local browser were provided to confirm script behaves identically to the existing web page: https://www.hackforla.org/design-system/components/button.html

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 jphamtv-fix-loop-variable-scope-5706 gh-pages
git pull https://github.com/jphamtv/website.git fix-loop-variable-scope-5706

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/jphamtv/website/blob/fix-loop-variable-scope-5706/CONTRIBUTING.md  

@agutiernc
Copy link
Member

Availability: 11/29 - after 8pm
Review ETA: 11/29 - EOD

@Thinking-Panda Thinking-Panda self-requested a review November 29, 2023 03:01
@Thinking-Panda
Copy link
Member

Availability: weekdays
ETA: 11/29

Copy link
Member

@Thinking-Panda Thinking-Panda left a comment

Choose a reason for hiding this comment

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

@jphamtv Thank you for working on this issue. Merge branches look good. When locally visited, website works identical to the published website. Well done!

Copy link
Member

@agutiernc agutiernc left a comment

Choose a reason for hiding this comment

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

Great work resolving this issue @jphamtv. Everything looks good as it has the correct fixed link, branches, and the variable was declared properly. The functionality of the website is the same. Thanks for contributing!

@kwangric kwangric merged commit 3d2b736 into hackforla:gh-pages Nov 30, 2023
@jphamtv jphamtv deleted the fix-loop-variable-scope-5706 branch November 30, 2023 21:58
@roslynwythe roslynwythe mentioned this pull request Feb 20, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve CodeQL Alert 41: Missing variable declaration
4 participants