Skip to content

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

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

Merged
SamTV12345 merged 1 commit into from
Jul 23, 2026
Merged

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

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
18 changes: 18 additions & 0 deletions playwright/specs/sheet_excel_chrome.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ test.describe('Sheet Excel chrome', () => {
await expect(cell(page, 0, 0)).toHaveCSS('font-size', /^26\.6/);
});

test('Ctrl+B/I/U toggle bold, italic, underline on the selection', async ({ page }) => {
const padId = `xl-fmtkeys-${Date.now()}`;
await openSheet(page, padId);
await commitCell(page, 0, 0, 'x'); // A1
await cell(page, 0, 0).click();

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/);
Comment on lines +60 to +69

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Jul 23, 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.

Informational

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

});

test('wrap text switches the cell to normal white-space', async ({ page }) => {
const padId = `xl-wrap-${Date.now()}`;
await openSheet(page, padId);
Expand Down
18 changes: 18 additions & 0 deletions ui/src/js/sheet/sheetEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,24 @@ export function startSheetEditor(root: HTMLElement): void {
doPaste();
return;
}
// Ctrl/Cmd+B/I/U toggle the style on the selection, mirroring the ribbon's
// toggle buttons. Applying a style blurs the active cell, so the next
// shortcut arrives with focus on <body> — we must NOT require grid focus
// (that would break chaining B then I). Instead just skip real form fields
// so the formula bar keeps these keys. preventDefault stops the browser's
// contenteditable rich-text default on the focused cell.
const tag = (e.target as HTMLElement | null)?.tagName;
const inField = tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT';
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;
Comment on lines +584 to +591

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Jul 23, 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.

Remediation recommended

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

}
}
// Clear the selection (single cell or range), like Excel. The grid-focus
// guard replaces the old single-cell exclusion: it lets Delete clear one
// cell while still keeping Backspace working in the formula bar and any
Expand Down
Loading