Accessibility improvements for screen reader users#316
Accessibility improvements for screen reader users#316ogomez92 wants to merge 8 commits intoTrimps:masterfrom
Conversation
|
Hi, this was mostly vibe coed. Not sure how active this project is (no updates in 11 months!) but if you try it out and this breaks your layout, feel free to ignore or take part of the changes. the screenreader mode was being very left out. You can find my fork at https://trimps.oriolgomez.com |
There was a problem hiding this comment.
Pull request overview
This PR targets a broad accessibility upgrade for the Screen Reader layout, improving semantics (buttons/radios), ARIA live announcements, and SR-specific UI behavior across the game client.
Changes:
- Updated
ScreenReader.htmlto use more semantic controls (notably<button>and radio-group fieldsets) and added SR live regions for log + zone announcements. - Added SR-specific behavior in JS (announcements for events, SR-friendly export flow, SR log announcing, and radio-state syncing).
- Adjusted defaults/config related to Screen Reader info visibility and tooltip interaction mode.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
updates.js |
Adds SR-specific export/log announcements and SR-friendly “per second / max resource” announcements + buy tab radio syncing. |
main.js |
Adds SR event announcements, SR grid rendering as tables, SR tooltip/info-button helpers, and SR import selection radio behavior. |
config.js |
Changes default showSRInfo setting and ensures SR info button creation on AutoStorage unlock. |
ScreenReader.html |
Reworks markup for SR layout (buttons/radios/ARIA live regions) and adds roles/tabindex for remaining non-button controls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ScreenReader.html
Outdated
| </div> | ||
| </div> | ||
| <div class="btn btn-danger inPortalBtn" id="closeStatsBtn" onclick="toggleStats()">Close</div> <div class="btn btn-info inPortalBtn" id="closeStatsBtn" onclick="tooltip('Trimps Info', null, 'update')">Trimps Info</div> | ||
| <button class="btn btn-danger inPortalBtn" id="closeStatsBtn" onclick="toggleStats()">Close</button> <button class="btn btn-info inPortalBtn" id="closeStatsBtn" onclick="tooltip('Trimps Info', null, 'update')">Trimps Info</button> |
There was a problem hiding this comment.
Duplicate id="closeStatsBtn" is used for two different buttons on the same line. Duplicate IDs break getElementById and can cause event binding/tooltips to attach to the wrong element. Give each button a unique id (e.g., closeStatsBtn and trimpsInfoBtn).
| <button class="btn btn-danger inPortalBtn" id="closeStatsBtn" onclick="toggleStats()">Close</button> <button class="btn btn-info inPortalBtn" id="closeStatsBtn" onclick="tooltip('Trimps Info', null, 'update')">Trimps Info</button> | |
| <button class="btn btn-danger inPortalBtn" id="closeStatsBtn" onclick="toggleStats()">Close</button> <button class="btn btn-info inPortalBtn" id="trimpsInfoBtn" onclick="tooltip('Trimps Info', null, 'update')">Trimps Info</button> |
There was a problem hiding this comment.
Fixed — renamed to id="trimpsInfoBtn".
main.js
Outdated
| //every minute, grant a bone | ||
| if (loops % 600 == 0){ | ||
| game.global.b++; | ||
| updateSkeleBtn(); | ||
| } |
There was a problem hiding this comment.
gameLoop now increments game.global.b every minute. This changes core gameplay/economy and isn’t related to the accessibility scope described in the PR; it also bypasses the existing Skeletimp-based bone mechanics. Please remove this, or gate it behind an explicit debug/cheat flag that is not enabled in production.
| if (game.unlocks.imps[item]){ | ||
| row.className = 'importOwned'; | ||
| if (usingScreenReader && which == 0) { | ||
| var row = elem.insertRow(); |
There was a problem hiding this comment.
In the usingScreenReader && which == 0 branch, the inserted row/cell never gets row.id = item (unlike the non-screenreader branch). selectImp() later calls document.getElementById(name) and will error / fail to highlight the selection because that element doesn’t exist. Set a stable id for the selectable row/label (or update selectImp to target the radio/label) so SR imports remain functional.
| var row = elem.insertRow(); | |
| var row = elem.insertRow(); | |
| row.id = item; |
There was a problem hiding this comment.
Already fixed in a prior commit — row.id = item is set on the next line.
| var a = document.createElement('a'); | ||
| a.download = saveName + '.txt'; | ||
| if (Blob !== null) { | ||
| var blob = new Blob([saveText], {type: 'text/plain'}); | ||
| a.href = URL.createObjectURL(blob); | ||
| } else { | ||
| a.href = 'data:text/plain,' + encodeURIComponent(saveText); |
There was a problem hiding this comment.
The screen reader Export auto-download path checks if (Blob !== null); if Blob is undefined, this will throw a ReferenceError before the check runs. Use a typeof Blob !== 'undefined' (and URL && URL.createObjectURL) style guard, and revoke any created object URL after triggering the download to avoid leaking object URLs over repeated exports.
There was a problem hiding this comment.
Fixed — now uses typeof Blob/URL guards and calls URL.revokeObjectURL() after the download.
| textString += "</tbody></table>"; | ||
| if (usingScreenReader) { | ||
| var resName = what.charAt(0).toUpperCase() + what.slice(1); | ||
| screenReaderAssert(resName + " per second: " + prettify(currentCalc)); |
There was a problem hiding this comment.
In screen reader mode getPsString returns early after announcing, but it no longer clears game.global.lockTooltip like the tooltip path does. This can leave the UI stuck in a locked-tooltip state if lockTooltip was set when the user triggered this. Clear/unlock the tooltip state (e.g., via unlockTooltip() or setting game.global.lockTooltip = false) before returning.
| screenReaderAssert(resName + " per second: " + prettify(currentCalc)); | |
| screenReaderAssert(resName + " per second: " + prettify(currentCalc)); | |
| game.global.lockTooltip = false; |
There was a problem hiding this comment.
Fixed — added game.global.lockTooltip = false; before the early return.
ScreenReader.html
Outdated
| <button id="mapsBtn" class="btn btn-warning fightBtn" style="display: none"> | ||
| <div id='mapsBtnText' onclick="mapsClicked()">Maps</div> | ||
| <button id='mapsBtnText' onclick="mapsClicked()">Maps</button> | ||
| <button aria-label='Configure Map Settings. Press S to view the popup.' onclick="tooltip('Configure Maps', null, 'update')"> | ||
| <span class='glyphicon glyphicon-cog'>Configure Maps</span></button> | ||
| </button> |
There was a problem hiding this comment.
Invalid HTML: a <button> is nested inside another <button> (#mapsBtn contains #mapsBtnText and the config button). Nested interactive elements are not allowed and can break focus/activation for assistive tech. Restructure this as a single button, or use a non-interactive wrapper (e.g., <div>) containing separate sibling buttons.
There was a problem hiding this comment.
Already fixed in a prior commit — #mapsBtn is now a <div> wrapper, not a <button>.
ScreenReader.html
Outdated
| <td class="btn btn-info" onclick="save(false, true)" role="link" tabindex="0"> | ||
| Save <span id="saveIndicator"></span><span id="playFabIndicator"></span> | ||
| </td> | ||
| <td class="btn btn-info" onclick="tooltip('Export', null, 'update')"> | ||
| <td class="btn btn-info" onclick="tooltip('Export', null, 'update')" role="link" tabindex="0"> | ||
| <div>Export</div> |
There was a problem hiding this comment.
These td elements are given role="link" + tabindex="0", but onclick won’t fire from keyboard activation by default on a td. Screen reader/keyboard users won’t be able to activate Save/Export/Import/etc. reliably. Convert these controls to real <button>/<a> elements, or add keydown handlers for Enter/Space to trigger the same action.
There was a problem hiding this comment.
Fixed — changed role="link" to role="button" and added onkeydown handlers for Enter/Space on all settings table cells.
ScreenReader.html
Outdated
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab2"> | ||
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab3"> | ||
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> |
There was a problem hiding this comment.
Superfluous argument passed to function presetTab.
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> | |
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3)"> <span id="presetTab3Text">Preset 3</span> |
There was a problem hiding this comment.
Fixed — removed the superfluous second argument from all three presetTab() calls.
ScreenReader.html
Outdated
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab2"> | ||
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab3"> | ||
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> |
There was a problem hiding this comment.
Superfluous argument passed to function presetTab.
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> | |
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3)"> <span id="presetTab3Text">Preset 3</span> |
ScreenReader.html
Outdated
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab2"> | ||
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | ||
| </label> | ||
| <label class="tabNotSelected" id="presetTab3"> | ||
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> |
There was a problem hiding this comment.
Superfluous argument passed to function presetTab.
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1, true)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2, true)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3, true)"> <span id="presetTab3Text">Preset 3</span> | |
| <input type="radio" name="perkPreset" value="1" onclick="presetTab(1)"> <span id="presetTab1Text">Preset 1</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab2"> | |
| <input type="radio" name="perkPreset" value="2" onclick="presetTab(2)"> <span id="presetTab2Text">Preset 2</span> | |
| </label> | |
| <label class="tabNotSelected" id="presetTab3"> | |
| <input type="radio" name="perkPreset" value="3" onclick="presetTab(3)"> <span id="presetTab3Text">Preset 3</span> |
…eyboard a11y, remove bone timer - Fix duplicate id="closeStatsBtn" (renamed to trimpsInfoBtn) - Use typeof Blob/URL guards + revokeObjectURL in SR export - Clear lockTooltip in getPsString/getMaxResources SR paths - Add keyboard handlers (Enter/Space) to settings table buttons - Change role="link" to role="button" on settings table cells - Remove superfluous second arg from presetTab() radio calls - Remove bone-per-5-minutes timer from gameLoop Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
There were big updates to the screen reader mode after the U2 update last year. I have played the game until I ran out of content. It's basically complete. At worst some minor quality of life things are missing |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Comprehensive accessibility (a11y) overhaul for the screen reader layout, spanning HTML semantics, ARIA attributes, and JavaScript behavior across 4 files (ScreenReader.html, config.js, main.js, updates.js).
HTML Semantics & ARIA (ScreenReader.html)
<div>/<span>elements with proper<button>elements throughout the UI — bone shop buttons, auto-toggle buttons, map/battle controls, settings buttons, heirloom actions, tutorial navigation, and more<ul>/<li>to<fieldset>/<label>/<input type="radio">for proper form semantics and screen reader navigationrole="tab"to remaining tab list items (buy tabs, feature tabs, settings tabs)role="button"andtabindex="0"to interactive elements that remain as<div>/<td>(battle stat breakdowns, health bar, settings bar cells)aria-labelattributes to icon-only buttons (close buttons, loot breakdown, tutorial nav, heirloom rarity arrows, etc.)srLogAnnounce(polite) for game log messages,srZoneAnnounce(assertive) for zone transitionsrole="log"witharia-live="off"to the game log container (announcements handled via the separate live region to avoid excessive verbosity)<h1>headings, improving navigation flow<span>elements to<button>elementsJavaScript Accessibility Improvements (main.js, updates.js)
screenReaderAssert) for key game events: zone transitions, building construction, void buff activation/expiration, Anticipation stacking/consumption, Titimp activation/expiration, Sugar Rush activation/expiration, Well Fed (Turkimp) activation/expiration<table role="grid">with row headers and cell metadata (mutations, empowerment tokens, loot icons, egg cells) for screen reader usersaria-current="true"and a visual*prefix for screen reader identification<h1>tags so screen reader users can quickly jump between purchasable items using heading navigationmakeAccessibleTooltipto usecreateSRInfoButton()andensureSRInfoButton()helpers, supporting deferred creation for initially-hidden elements (e.g., AutoStorage button). Info buttons now userole="button",tabindex="0", and keyboard event handlersnumTab()andpresetTab()now sync the checked state of corresponding radio inputs in the screen reader layout<button>elements withdisabledattribute when unavailable; descriptions are inlined into button text; purchases skip the confirmation tooltip modal for directness.txtfile instead of showing a modal with a textareagetPsString()andgetMaxResources()announce values viascreenReaderAssertinstead of opening tooltip modalsinsertAdjacentHTMLinstead ofinnerHTML +=and routes new messages through thesrLogAnnouncelive regionsrTooltipModefrom"click"to"button"for better screen reader discoverabilityshowSRInfosetting to off (hidden by default)Test plan
🤖 Generated with Claude Code