feat(sheet): Ctrl+B/I/U toggle bold, italic, underline

[SamTV12345]

What

Keyboard shortcuts Ctrl/Cmd+B / +I / +U to toggle bold, italic, underline on the current selection — mirroring the ribbon's existing toggle buttons.

How

Added to the document keydown handler. Two deliberate guard choices:

• Skip form fields (INPUT/TEXTAREA/SELECT) so the formula bar keeps these keys — but do not require grid focus, because applying a style blurs the active cell; requiring focus would break chaining (press B, then I).
• preventDefault stops the browser's native contenteditable rich-text formatting on the focused cell.

Toggle reads the focus cell's prop (bold/italic/underline = '1') and flips it via the same applyStyleToSelection the buttons use.

Tests

• New sheet_excel_chrome.spec.ts test: B→bold, I→italic, U→underline, then B again → back to normal.
• Ran the full sheet_excel_chrome + sheet_selection specs locally (15 tests) — all pass, incl. the existing Delete/formula-bar tests (shared keydown handler, no regression).

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Sheet: Add Ctrl/Cmd B/I/U formatting shortcuts

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

AI Description

• Add Ctrl/Cmd+B/I/U shortcuts to toggle bold/italic/underline on the selection.
• Guard against interfering with formula bar inputs and browser contenteditable defaults.
• Add Playwright coverage to verify toggling behavior and regression safety.

Diagram

graph TD
  A["User presses Ctrl/Cmd+B/I/U"] --> B["Document keydown handler"] --> C["Guard checks (not editing, not read-only, not form field)"] --> D["Toggle style key (bold/italic/underline)"] --> E["applyStyleToSelection"] --> F["Grid cell props/rendering"]
  T["Playwright spec"] --> A

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize shortcuts in a keybinding map/registry

• ➕ Keeps the keydown handler from growing as more shortcuts are added
• ➕ Easier to test/inspect shortcut conflicts in one place
• ➕ Enables consistent platform-specific handling (Ctrl vs Cmd) and discoverability
• ➖ More refactor upfront for a small feature
• ➖ May require additional abstraction to access sheet state (editing/read-only/selection)

2. Dispatch to existing ribbon toggle actions/commands

• ➕ Single source of truth for formatting behavior (UI buttons and shortcuts)
• ➕ Reduces risk of divergence in edge cases (multi-cell selections, mixed states)
• ➖ Requires a command layer or exported action surface if it doesn’t exist today
• ➖ Might be more intrusive than directly calling applyStyleToSelection

Recommendation: The current approach is appropriate for a small, targeted addition: it reuses applyStyleToSelection (same behavior as ribbon toggles), prevents native browser formatting, and deliberately avoids a grid-focus requirement to allow chaining after focus changes. If shortcut scope expands (more formatting keys), consider moving to a centralized shortcut registry or command dispatch to keep the keydown handler maintainable and to manage conflicts systematically.

Files changed (2) +36 / -0

Enhancement (1) +18 / -0

sheetEditor.tsHandle Ctrl/Cmd B/I/U to toggle bold/italic/underline on selection +18/-0

Handle Ctrl/Cmd B/I/U to toggle bold/italic/underline on selection

• Extends the document keydown handler to map Ctrl/Cmd+B/I/U into bold/italic/underline toggles using applyStyleToSelection. Adds guards to avoid interfering with form fields (formula bar) and calls preventDefault to stop browser contenteditable rich-text behavior.

ui/src/js/sheet/sheetEditor.ts

Tests (1) +18 / -0

sheet_excel_chrome.spec.tsAdd Playwright test for Ctrl/Cmd B/I/U formatting toggles +18/-0

Add Playwright test for Ctrl/Cmd B/I/U formatting toggles

• Adds an end-to-end test that opens a sheet, selects a cell, and verifies Ctrl+B/I/U toggles bold/italic/underline via computed CSS. Also verifies pressing Ctrl+B again turns bold back off.

playwright/specs/sheet_excel_chrome.spec.ts

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Repeat toggles style twice 🐞 Bug ≡ Correctness

Description

The Ctrl/Cmd+B/I/U handler runs on every keydown event without checking KeyboardEvent.repeat, so
holding the shortcut can flip the style multiple times and leave the selection in the wrong final
state.

Code

ui/src/js/sheet/sheetEditor.ts[R584-591]

+    if (mod && !editingNow() && !readOnly && !inField) {
+      const k = e.key.toLowerCase();
+      const styleKey = k === 'b' ? 'bold' : k === 'i' ? 'italic' : k === 'u' ? 'underline' : null;
+      if (styleKey) {
+        e.preventDefault();
+        const on = propsOf(selection.focus.row, selection.focus.col)[styleKey] === '1';
+        applyStyleToSelection({ [styleKey]: on ? '' : '1' });
+        return;

Evidence

The document keydown listener applies the toggle immediately when styleKey matches, with no guard
against repeated keydown events (e.repeat).

ui/src/js/sheet/sheetEditor.ts[558-593]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The Ctrl/Cmd+B/I/U shortcut toggles style on *every* `keydown`, including auto-repeat events when the key is held. This can cause multiple toggles and an unintended final style state.

### Issue Context
The handler is attached at the document level and calls `applyStyleToSelection()` immediately when it detects Ctrl/Cmd + (b/i/u).

### Fix Focus Areas
- Add an `e.repeat` guard so toggles only apply once per physical key press.

- ui/src/js/sheet/sheetEditor.ts[558-593]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Cmd path untested 🐞 Bug ☼ Reliability

Description

The implementation supports Cmd shortcuts via e.metaKey, but the new Playwright test only presses
Control+b/i/u, so it will not catch regressions specific to the metaKey path.

Code

playwright/specs/sheet_excel_chrome.spec.ts[R60-69]

+    await page.keyboard.press('Control+b');
+    await expect(cell(page, 0, 0)).toHaveCSS('font-weight', /700|bold/);
+    await page.keyboard.press('Control+i');
+    await expect(cell(page, 0, 0)).toHaveCSS('font-style', 'italic');
+    await page.keyboard.press('Control+u');
+    await expect(cell(page, 0, 0)).toHaveCSS('text-decoration', /underline/);
+
+    // Ctrl+B again toggles bold back off.
+    await page.keyboard.press('Control+b');
+    await expect(cell(page, 0, 0)).toHaveCSS('font-weight', /400|normal/);

Evidence

The handler explicitly treats Ctrl and Meta as the modifier, but the test only sends Control-based
shortcuts.

ui/src/js/sheet/sheetEditor.ts[558-586]
playwright/specs/sheet_excel_chrome.spec.ts[54-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The shortcut handler treats Ctrl and Cmd equivalently (`e.ctrlKey || e.metaKey`), but the added e2e test only exercises the Ctrl path.

### Issue Context
This is a coverage gap: it doesn't prove Cmd is broken, but it reduces confidence for macOS users.

### Fix Focus Areas
- Add a variant that presses `Meta+b/i/u` (or parameterize by modifier), ideally in a way that won't be flaky on non-mac runners.

- playwright/specs/sheet_excel_chrome.spec.ts[54-70]
- ui/src/js/sheet/sheetEditor.ts[558-593]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

1. Repeat toggles style twice 🐞 Bug ≡ Correctness

The Ctrl/Cmd+B/I/U handler runs on every keydown event without checking KeyboardEvent.repeat, so
holding the shortcut can flip the style multiple times and leave the selection in the wrong final
state.

Agent Prompt

### Issue description
The Ctrl/Cmd+B/I/U shortcut toggles style on *every* `keydown`, including auto-repeat events when the key is held. This can cause multiple toggles and an unintended final style state.

### Issue Context
The handler is attached at the document level and calls `applyStyleToSelection()` immediately when it detects Ctrl/Cmd + (b/i/u).

### Fix Focus Areas
- Add an `e.repeat` guard so toggles only apply once per physical key press.

- ui/src/js/sheet/sheetEditor.ts[558-593]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

2. Cmd path untested 🐞 Bug ☼ Reliability

The implementation supports Cmd shortcuts via e.metaKey, but the new Playwright test only presses
Control+b/i/u, so it will not catch regressions specific to the metaKey path.

Agent Prompt

### Issue description
The shortcut handler treats Ctrl and Cmd equivalently (`e.ctrlKey || e.metaKey`), but the added e2e test only exercises the Ctrl path.

### Issue Context
This is a coverage gap: it doesn't prove Cmd is broken, but it reduces confidence for macOS users.

### Fix Focus Areas
- Add a variant that presses `Meta+b/i/u` (or parameterize by modifier), ideally in a way that won't be flaky on non-mac runners.

- playwright/specs/sheet_excel_chrome.spec.ts[54-70]
- ui/src/js/sheet/sheetEditor.ts[558-593]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools