Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #465 by converting navigation elements from buttons with click event handlers that change location.href to semantic anchor (<a>) elements with proper href attributes. This improves accessibility by enabling right-click context menus (open in new tab, copy link, etc.) and better screen reader support while maintaining the same visual styling and functionality.
Changes:
- Converted button elements with navigation click handlers to anchor elements with href attributes across 22 files
- Added
rel="noopener noreferrer"to external links withtarget="_blank"for security - Added aria-label attributes to icon-only navigation links for better accessibility
- Removed unnecessary
role="button"andtabindex="0"attributes from semantic anchor elements
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| interfaces/quicktype/index.html | Converted project management button to anchor, added styling |
| interfaces/project/options.html | Converted project management button to anchor, added styling |
| interfaces/project/metadata.html | Converted project management button to anchor, added styling |
| interfaces/project/index.html | Added aria-label to icon-only management link |
| interfaces/manifest-import/examples.html | Added security attributes to external link |
| interfaces/manage-project/index.html | Removed role/tabindex from footer navigation links |
| interfaces/manage-project/collaborators.html | Converted project management button to anchor, added styling |
| interfaces/manage-layers/index.html | Converted project management button to anchor, added styling |
| interfaces/manage-columns/index.js | Converted three navigation buttons to anchors, added styling |
| interfaces/import-tpen28/index.js | Removed click handler, set href directly |
| interfaces/import-tpen28/index.html | Converted button element to anchor |
| interfaces/import-tpen28/index.css | Added a.btn styling to match button appearance |
| components/user-profile/index.js | Converted social media icon onclick to proper anchor links |
| components/user-profile/contributionActivity.js | Converted contribution icon onclick to proper anchor links |
| components/projects/list-navigation.js | Added aria-label to icon-only management link |
| components/project-options/index.js | Added aria-labels to icon-only edit links |
| components/project-export/index.js | Added security attributes to external IIIF links |
| components/page-tool/index.js | Converted lines/columns buttons to anchors, added styling |
| components/manage-role/index.js | Converted project management button to anchor, added styling |
| components/leave-project/index.js | Converted cancel button to anchor with href |
| components/gui/site/Footer.js | Added security attributes to license link |
| components/annotorious-annotator/line-parser.js | Converted navigation buttons to anchors with helpful comment about OSD exception |
| about.html | Added security attributes to external link |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cubap
left a comment
There was a problem hiding this comment.
I'm sure I haven't managed to look at everything, but I didn't make it break. There are a few places that look a little different but not wrong.
|
I noticed here that buttons on project management and possible other places don't have pointers. Something is off with cursor but not from this PR. |
Closes #465
Summary
Converts navigation elements across the codebase from
<button>withlocation.hrefclick handlers and<img onclick="window.open(...)">to proper<a href>elements. This gives users consistent browser link features everywhere: right-click context menu, middle-click / Ctrl+click to open in new tab, status bar URL preview, and correct screen reader announcements.Changes
<button>→<a>conversions (with CSS preservation)options.html,metadata.html,collaborators.html,quicktype/index.html,manage-layers/index.html): "Go to Project Management" buttonsline-parser.js:#createColumnsBtnand#projectManagementBtnmanage-role/index.js:#projectManagementBtnpage-tool/index.js:.lines-btnand.columns-btnmanage-columns/index.js: 3.goBtnnavigation elementsleave-project/index.js:#noLeaveBtn(href set directly in template)import-tpen28:#openProjectbuttonEach conversion adds
text-decoration: none; font: inherit;(anddisplay: inline-block,box-sizing: border-box,text-align: centerwhere needed) to preserve the button appearance on<a>elements.<img onclick="window.open(...)">→<a href>wrappinguser-profile/index.js: 6 social media iconscontributionActivity.js: 4 RERUM/Transcribe icons (initial render + "show more")Security:
rel="noopener noreferrer"ontarget="_blank"linksFooter.js,project-export/index.js(4 instances),about.html,manifest-import/examples.htmlAccessibility
aria-labelto icon-only links:project-options/index.js(2 emoji links),list-navigation.js(gear icon),project/index.html(gear icon)role="button"and redundanttabindex="0"from 6<a>footer elements inmanage-project/index.htmlDocumented exceptions
line-parser.js: OpenSeadragon "Go Transcribe" button stays as OSD Button (the API creates<div>elements, not<a>)Header.js: Logout button stays as<button>becauseTPEN.logout()clearslocalStoragebefore redirecting. An<a href>would allow middle-click / right-click "Open in new tab" to hit the logout URL without clearing the token in the current tab, leaving a broken state.new-action.jsandproject/create.html: TPEN 2.8 import buttons stay as<button>because they setdocument.cookiewith a TPEN 2.8 auth cookie before redirecting to the TPEN 2.8 login page. An<a href>would skip the cookie setup on middle-click / right-click, causing the login to fail.Not in this PR
The Header action-link (
tpen-action-link/.action-button) remains a<button>with callback. Converting it requires changing thetpen-gui-action-linkevent contract from{label, callback}to{label, href}, which will be addressed in a separate PR.Test plan
jekyll s— run locally<a>elements)