Major UX changes for workshops website#604
Conversation
Co-authored-by: Oliver Zhang <[email protected]>
Co-authored-by: Oliver Zhang <[email protected]>
Co-authored-by: Oliver Zhang <[email protected]>
Co-authored-by: Oliver Zhang <[email protected]>
* Add videos from Hack 2024. (#574) * Fix Python Turtle workshops (#576) * Fix Python Turtle workshops. (#577) * Revert "Fix Python Turtle workshops (#576)" This reverts commit a19b792. * Add answers to extra exercise in activity 3 in Python Turtle. (#578) * Fix images in HTML workshop (#580) * Fix images again for HTML workshop * Add layout4 shortcode * Update menu formatting
#575) (#584) * UI/UX changes in line with spec: back button from workshop modules to dashboard, right-aligned primary panel, black bar at the end of the body text. * Removed console logs, added localization. * Fixed build error. * forgot to save changes related to removing console logs in script --------- Co-authored-by: Kourosh Arfania <[email protected]> Co-authored-by: Kory Arfania <[email protected]>
Removed home section for Workshop Projects from layout.
…structions and first mission
| </select> | ||
| {{- end }} No newline at end of file | ||
| {{if not .IsHome}} | ||
| {{- if and hugo.IsMultilingual (not .Site.Params.DisableLanguageSwitchingButton)}} |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Copilot Autofix
AI about 1 month ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| {{end}} | ||
|
|
||
| {{if .IsHome}} | ||
| {{- if and hugo.IsMultilingual (not .Site.Params.DisableLanguageSwitchingButton)}} |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, the fix is to avoid directly using this.value from the DOM in a sensitive sink (navigation) without any validation or restriction. Instead, use a small piece of JavaScript that reads the selected value, validates it (e.g., ensures it is an HTTP(S) URL or a same‑origin path), and only then sets window.location. This also removes the inline event handler, reducing XSS surface.
For this specific file, we can keep existing functionality (navigating to the selected language’s permalink) while making it safer by:
- Removing the inline
onchange="location = this.value;"attributes from both<select>elements. - Adding a small
<script>block right after the second<select>that:- Looks up both
#select-language-workshopand#select-language-homeif present. - Attaches a
changeevent listener to each. - In the handler, reads
event.target.value, checks that it begins with/(relative path) orhttp://orhttps://(or more restrictive, if desired), and only then assigns it towindow.location.href. Invalid values are ignored.
- Looks up both
This preserves current behavior for valid permalinks but ensures that even if a malicious value reached the DOM, obviously dangerous schemes (like javascript:) would not be used for navigation. All changes occur in themes/docdock/layouts/partials/language-selector.html, within the shown snippet.
| @@ -1,6 +1,6 @@ | ||
| {{if not .IsHome}} | ||
| {{- if and hugo.IsMultilingual (not .Site.Params.DisableLanguageSwitchingButton)}} | ||
| <select id="select-language-workshop" onchange="location = this.value;"> | ||
| <select id="select-language-workshop"> | ||
| {{ $siteLanguages := .Site.Languages}} | ||
| {{ $pageLang := .Page.Lang}} | ||
| {{ range .Page.AllTranslations }} | ||
| @@ -21,7 +21,7 @@ | ||
|
|
||
| {{if .IsHome}} | ||
| {{- if and hugo.IsMultilingual (not .Site.Params.DisableLanguageSwitchingButton)}} | ||
| <select id="select-language-home" onchange="location = this.value;"> | ||
| <select id="select-language-home"> | ||
| {{ $siteLanguages := .Site.Languages}} | ||
| {{ $pageLang := .Page.Lang}} | ||
| {{ range .Page.AllTranslations }} | ||
| @@ -37,5 +37,32 @@ | ||
| {{ end }} | ||
| {{ end }} | ||
| </select> | ||
| <script> | ||
| (function () { | ||
| function isSafeUrl(url) { | ||
| if (!url) return false; | ||
| // Allow same-site relative paths or explicit http/https URLs | ||
| return url.charAt(0) === '/' || | ||
| url.indexOf('http://') === 0 || | ||
| url.indexOf('https://') === 0; | ||
| } | ||
|
|
||
| function attachLanguageSelectorHandler(selectId) { | ||
| var select = document.getElementById(selectId); | ||
| if (!select) return; | ||
| select.addEventListener('change', function (event) { | ||
| var target = event.target || event.srcElement; | ||
| if (!target) return; | ||
| var value = target.value; | ||
| if (isSafeUrl(value)) { | ||
| window.location.href = value; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| attachLanguageSelectorHandler('select-language-workshop'); | ||
| attachLanguageSelectorHandler('select-language-home'); | ||
| })(); | ||
| </script> | ||
| {{- end }} | ||
| {{end}} |
|
Hi @ozhang22! This is a great set of UX improvements. Heads up that this PR has merge conflicts with the current master branch (likely from the changes in your merged PR #605 and our recent content fixes). Would you like to rebase this against master? If you'd like help, happy to assist. The auto-translation PR #607 was closed since it was based on this conflicted state - a fresh one will generate once this is rebased and ready. |
|
Hi @ozhang22! After reviewing the full PR queue, here is what I found on this PR: Your UX changes from PR #605 (24 files) are already merged. This PR #604 has 98 additional files that didn't make it into #605:
Since this PR has merge conflicts with current master, here are two options: Option A (recommended): Close this PR and extract the chatbot rewrite into a fresh, focused PR from current master. The Spanish translations can be reconciled with #585. Option B: Rebase this against master (will need conflict resolution across 134 files). Happy to help with either approach! The chatbot content looks great and is worth preserving. |
|
Update: After deeper analysis, all unique content from this PR has been extracted into clean, conflict-free PRs:
This PR can now be safely closed. All content is preserved in focused PRs that merge cleanly against current master. Thank you @ozhang22 for all this work! |
Extract two new Spanish-language workshops from NuevoFoundation#604: Aventura React (34 files): - Costa Rica-themed React tutorial teaching components, props, and styling - 8 activities + answer key with original illustrations and media Logic Gates (47 files): - Costa Rica-themed logic circuit simulator workshop - 8 activities covering AND, OR, NOT gates with interactive simulator - Custom CSS, illustrations, and answer key with solutions Original content created in NuevoFoundation#604 by @ozhang22 and contributors. Extracted into focused PR to avoid conflicts in the larger NuevoFoundation#604. Co-authored-by: Oliver Zhang <[email protected]> Co-authored-by: Copilot <[email protected]>
No description provided.