Skip to content

🎨 Palette: Improve Staff Access Modal UX & Accessibility #38

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.

Sign up for GitHub

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

Open
LVT-ENG wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🎨 Palette: Improve Staff Access Modal UX & Accessibility #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .jules/palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2025-05-15 - Staff Access Accessibility & UX
**Learning:** Adding `aria-haspopup="dialog"` and `role="button"` to triggers for modal dialogs is essential for screen readers to properly announce the interaction. Focus management (auto-focusing the first input) and keyboard support (Enter key) are critical for power users and those with motor impairments.
**Action:** Always ensure modal triggers have appropriate ARIA roles and that modals implement focus management and keyboard listeners for common actions.
4 changes: 2 additions & 2 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ <h1>TRYONYOU</h1>
<li><a href="#features">Experiencia</a></li>
<li><a href="#try-on">Búnker</a></li>
<li><a href="#consultation">P.A.U.</a></li>
<li><a href="javascript:void(0)" onclick="requestPrivatePass()">Staff Access</a></li>
<li><a href="javascript:void(0)" onclick="requestPrivatePass()" aria-haspopup="dialog" role="button">Staff Access</a></li>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 27, 2026

Choose a reason for hiding this comment

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

medium

For better accessibility and semantics, it's recommended to use a <button> element for actions instead of an <a> tag with role="button". A native <button> is keyboard-accessible by default (handling both Enter and Space keys) and is the correct semantic element for triggering an action like opening a modal.

You will need to apply styling to the button to make it visually consistent with the other navigation links. Using a class like nav-button can help with this.

Suggested change
<li><a href="javascript:void(0)" onclick="requestPrivatePass()" aria-haspopup="dialog" role="button">Staff Access</a></li>
<li><button type="button" class="nav-button" onclick="requestPrivatePass()" aria-haspopup="dialog">Staff Access</button></li>

</ul>
</nav>
</header>
Expand Down Expand Up @@ -123,7 +123,7 @@ <h3>Catálogo Shopify</h3>
<div class="modal-content" style="background:#141619; border:1px solid #C5A46D; padding:3rem; max-width:500px; text-align:center;">
<h3 style="color:#C5A46D; margin-bottom:1.5rem;">PASE PRIVADO SOLICITADO</h3>
<p style="color:#F5EFE6; opacity:0.7; margin-bottom:2rem;">Contenido restringido para Curadores del Búnker. Por favor, introduce tu credencial de acceso.</p>
<input type="password" id="private-pass-input" placeholder="Credential ID" style="width:100%; padding:1rem; background:#0d0e10; border:1px solid rgba(255,255,255,0.1); color:#F5EFE6; margin-bottom:2rem;">
<input type="password" id="private-pass-input" placeholder="Credential ID" aria-label="Staff Credential ID" style="width:100%; padding:1rem; background:#0d0e10; border:1px solid rgba(255,255,255,0.1); color:#F5EFE6; margin-bottom:2rem;">
<div style="display:flex; gap:1rem; justify-content:center;">
<button class="cta-button" onclick="verifyPrivatePass()" style="padding:10px 20px; font-size:0.8rem;">ACCEDER</button>
<button class="cta-button" onclick="closePrivatePass()" style="padding:10px 20px; font-size:0.8rem; background:transparent; border:1px solid #C5A46D; color:#C5A46D;">CERRAR</button>
Expand Down
12 changes: 12 additions & 0 deletions js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,19 @@ class TryOnYouBunker {

requestPrivatePass() {
const modal = document.getElementById('private-pass-modal');
const input = document.getElementById('private-pass-input');
modal.style.display = 'flex';
input.focus();

if (!this._passListener) {
input.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
e.preventDefault();
this.verifyPrivatePass();
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 27, 2026

Choose a reason for hiding this comment

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

security-critical critical

This function call leads to verifyPrivatePass, which performs password validation on the client-side with a hardcoded password ("SAC_MUSEUM_2026"). This is a critical security vulnerability. Anyone can view the page's JavaScript source to find the password and bypass this protection.

All authentication and authorization checks must be performed on the server-side. The client should send the password to a secure backend endpoint for verification.

}
});
this._passListener = true;
}
Comment on lines +237 to +245
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 27, 2026

Choose a reason for hiding this comment

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

medium

To improve code clarity, it's better to use a more descriptive name for the flag that tracks if the event listener has been added. _passListener is ambiguous; it could be a boolean or the listener function itself. A name like _isPassKeyListenerAttached would be more explicit.

For better maintainability and to prevent potential memory leaks, it's a best practice to add event listeners when they are needed and remove them when they are not (e.g., in closePrivatePass()). While the current implementation works, attaching an event listener that is never removed is not ideal in long-running applications.

Suggested change
if (!this._passListener) {
input.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
e.preventDefault();
this.verifyPrivatePass();
}
});
this._passListener = true;
}
if (!this._isPassKeyListenerAttached) {
input.addEventListener('keydown', (e) => {
if (e.key === 'Enter') {
e.preventDefault();
this.verifyPrivatePass();
}
});
this._isPassKeyListenerAttached = true;
}

}

closePrivatePass() {
Expand Down