Skip to content

🎨 Palette: Add keyboard shortcut hints and functionality to main action #138

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

Draft
Shin5hi wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

🎨 Palette: Add keyboard shortcut hints and functionality to main action #138

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
4 changes: 4 additions & 0 deletions .jules/palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2024-05-23 - Accessible Toast Notifications
**Learning:** Toast notifications often disappear too quickly for some users. Implementing a 5000ms minimum duration AND a manual close button ensures compliance with accessibility standards (WCAG 2.2.1 Timing Adjustable) and improves usability for everyone.
**Action:** When implementing temporary feedback messages, always include a visual close button and ensure the timeout is sufficient (>= 5000ms), or allow user preference to extend it.

## 2024-05-24 - Structural State Preservation for Keyboard Shortcuts
**Learning:** When implementing keyboard shortcuts (like Ctrl+K), adding a visual `<kbd>` hint creates nested HTML. Using `textContent` for saving/restoring the button state strips these HTML tags.
**Action:** Preserve structural integrity by saving `childNodes` (`Array.from(element.childNodes)`) and restoring them via `replaceChildren(...)`. This maintains elements like `<kbd>` without risking XSS via `innerHTML`.
20 changes: 14 additions & 6 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; style-src 'self' 'sha256-fgxmLOznNmVf4GAd24jy5Eiv/Rer+7sVLOBqnsVx0nY='; script-src 'self' 'sha256-cPRnZP+O4z5KsN+vdFstQwgKExGtoN98I3Gq+Tm1aSA='; object-src 'none'; base-uri 'self'; upgrade-insecure-requests;">
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; style-src 'self' 'sha256-fgxmLOznNmVf4GAd24jy5Eiv/Rer+7sVLOBqnsVx0nY='; script-src 'self' 'sha256-IsPUn2cDWu/zLkWBbuSD3J6kkmrVFt2CzB0ialDxvZs='; object-src 'none'; base-uri 'self'; upgrade-insecure-requests;">
<meta name="referrer" content="no-referrer">
<title>5ive - UX Sample</title>
<style>
Expand Down Expand Up @@ -109,8 +109,8 @@ <h1>Welcome to 5ive</h1>
<p>Sample accessible button component:</p>

<!-- ✅ GOOD UX: Semantic button with proper labeling and focus handling -->
<button type="button" class="btn" id="action-btn">
Click Me (Accessible)
<button type="button" class="btn" id="action-btn" aria-keyshortcuts="Control+K Meta+K">
Click Me (Accessible) <kbd>Ctrl+K</kbd>
</button>

<div id="feedback" class="feedback" aria-live="polite"></div>
Expand All @@ -123,6 +123,14 @@ <h1>Welcome to 5ive</h1>
const feedback = document.getElementById('feedback');
let feedbackTimeout;

document.addEventListener('keydown', (e) => {
if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k') {
e.preventDefault();
actionBtn.focus();
actionBtn.click();
}
});
Comment on lines +126 to +132

@coderabbitai coderabbitai Bot Apr 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Scope the global shortcut to avoid hijacking editable contexts.

At Line 127, this handler also fires while users are typing in editable elements and on combos like Ctrl/Cmd+Shift+K. That can trigger unintended actions and suppress expected defaults.

🔧 Proposed guard conditions 
 document.addEventListener('keydown', (e) => {
-  if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k') {
-    e.preventDefault();
-    actionBtn.focus();
-    actionBtn.click();
-  }
+  const isShortcut =
+    (e.ctrlKey || e.metaKey) &&
+    !e.altKey &&
+    !e.shiftKey &&
+    e.key.toLowerCase() === 'k';
+
+  const target = e.target;
+  const isEditable =
+    target instanceof HTMLElement &&
+    (target.isContentEditable ||
+      target.tagName === 'INPUT' ||
+      target.tagName === 'TEXTAREA' ||
+      target.tagName === 'SELECT');
+
+  if (!isShortcut || isEditable || actionBtn.disabled) return;
+
+  e.preventDefault();
+  actionBtn.focus();
+  actionBtn.click();
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
document.addEventListener('keydown', (e) => {
if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k') {
e.preventDefault();
actionBtn.focus();
actionBtn.click();
}
});
document.addEventListener('keydown', (e) => {
const isShortcut =
(e.ctrlKey || e.metaKey) &&
!e.altKey &&
!e.shiftKey &&
e.key.toLowerCase() === 'k';
const target = e.target;
const isEditable =
target instanceof HTMLElement &&
(target.isContentEditable ||
target.tagName === 'INPUT' ||
target.tagName === 'TEXTAREA' ||
target.tagName === 'SELECT');
if (!isShortcut || isEditable || actionBtn.disabled) return;
e.preventDefault();
actionBtn.focus();
actionBtn.click();
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@index.html` around lines 126 - 132, The global keydown handler on document
(the listener that checks (e.ctrlKey || e.metaKey) && e.key.toLowerCase() ===
'k' and then focuses and clicks actionBtn) needs guards so it doesn't run when
the user is typing or when other modifiers like Shift are held; update the
handler to return early if e.shiftKey is true and to return early when the event
target is an editable element (target.tagName is INPUT, TEXTAREA, or
target.isContentEditable is true) or a form field role, then proceed to
preventDefault(), focus actionBtn and click it only when those guards pass.


actionBtn.addEventListener('click', () => {
// Clear any existing timeout
if (feedbackTimeout) clearTimeout(feedbackTimeout);
Expand All @@ -133,10 +141,10 @@ <h1>Welcome to 5ive</h1>

// Set loading state
// ✅ Sentinel: Avoid innerHTML to prevent XSS
const originalText = actionBtn.textContent;
const originalNodes = Array.from(actionBtn.childNodes);
actionBtn.disabled = true;
actionBtn.setAttribute('aria-busy', 'true');
actionBtn.textContent = '';
actionBtn.replaceChildren();

const spinner = document.createElement('span');
spinner.className = 'spinner';
Expand All @@ -149,7 +157,7 @@ <h1>Welcome to 5ive</h1>
// Reset state
actionBtn.disabled = false;
actionBtn.removeAttribute('aria-busy');
actionBtn.textContent = originalText;
actionBtn.replaceChildren(...originalNodes);

// Show success with icon and transition
// ✅ Sentinel: Using textContent and DOM methods instead of innerHTML
Expand Down