Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/components/ToolbarButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ const ToolbarButton: FC<ToolbarButtonProps> = ({
{...longPress.props}
aria-label={command.label}
data-testid='toolbar-icon'
data-active={isButtonActive}
ref={dndRef(node => dragSource(dropTarget(node)))}
key={commandId}
title={`${command.label}${(command.overlay?.keyboard ?? command.keyboard) ? ` (${formatKeyboardShortcut((command.overlay?.keyboard ?? command.keyboard)!)})` : ''}${buttonError ? '\nError: ' + buttonError : ''}`}
Expand Down
27 changes: 26 additions & 1 deletion src/device/__tests__/selection.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import getTextContentFromHTML from '../../device/getTextContentFromHTML'
import { offsetFromClosestParent } from '../selection'
import { html, offsetFromClosestParent } from '../selection'

/** Create dummy div with given html value. */
const createDummyDiv = (htmlValue: string) => {
Expand Down Expand Up @@ -80,3 +80,28 @@
})
})
})

describe('html', () => {
// When the caret is collapsed on the editable element itself (e.g. when the cursor is moved to a thought by
// tapping its bullet), html() must return the editable's contents, not its outer wrapper element with attributes
// such as placeholder="<b>One</b>" (#3912).
it('returns the editable inner html rather than the wrapper element for a collapsed caret on the editable', () => {
const editable = document.createElement('div')
editable.setAttribute('contenteditable', 'true')
editable.setAttribute('data-editable', 'true')
editable.setAttribute('placeholder', '<b>One</b>')
editable.innerHTML = '<b>One</b>'
document.body.appendChild(editable)

const range = document.createRange()
range.setStart(editable, 0)
range.collapse(true)
const selection = window.getSelection()!
selection.removeAllRanges()
selection.addRange(range)

expect(html()).toBe('<b></b>')

Check failure on line 103 in src/device/__tests__/selection.ts

View workflow job for this annotation

GitHub Actions / TDD — Unit tests

[unit] src/device/__tests__/selection.ts > html > returns the editable inner html rather than the wrapper element for a collapsed caret on the editable

AssertionError: expected '<div contenteditable="true" data-edit…' to be '<b></b>' // Object.is equality Expected: "<b></b>" Received: "<div contenteditable="true" data-editable="true" placeholder="<b></b>"><b>One</b></div>" ❯ src/device/__tests__/selection.ts:103:20

document.body.removeChild(editable)
})
})
6 changes: 5 additions & 1 deletion src/device/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,11 @@ export const html = () => {

// Check if the node is an Element using the instanceof operator
if (node instanceof Element) {
containerHtml = node.outerHTML
// When the caret is collapsed on the editable element itself (e.g. when the cursor is moved to a thought
// by tapping its bullet), return the editable's inner HTML rather than its outerHTML, so that the wrapper
// element and its attributes (such as placeholder="<b>…</b>", whose value contains raw HTML) are excluded
// from the selection html (#3912).
containerHtml = node.getAttribute('contenteditable') === 'true' ? node.innerHTML : node.outerHTML
} else if (node instanceof CharacterData) {
while (node.parentElement?.tagName !== 'DIV') {
node = node.parentElement!
Expand Down
50 changes: 50 additions & 0 deletions src/e2e/puppeteer/__tests__/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,53 @@
const result = await getEditingText()
expect(result).toBe('<font color="#00c7e6">HELLO WORLD</font>')
})

/** Returns whether the Bold toolbar button is rendered in its active state. */
const isBoldButtonActive = () =>
page.evaluate(
() =>
document.querySelector('[data-testid="toolbar-icon"][aria-label="Bold"]')?.getAttribute('data-active') === 'true',
)

// Regression test for #3912: the Bold/Italic/Underline/Strikethrough buttons flickered back to their inactive
// state when the cursor was moved to a fully-formatted thought by tapping its bullet (Chrome/Android). Moving the
// cursor with a collapsed caret should reflect the whole-thought formatting, not a stale empty selection.
it('Bold button stays active when the cursor is moved to a fully-bold thought via its bullet (#3912)', async () => {
const importText = `
- One
- Two`

await paste(importText)

// format the whole first thought as bold
await clickThought('One')
await click('[data-testid="toolbar-icon"][aria-label="Bold"]')
await waitForEditable('<b>One</b>')

// move the cursor to the plain thought: the Bold button should be inactive
await clickThought('Two')
expect(await isBoldButtonActive()).toBe(false)

// move the cursor back to the bold thought by tapping its bullet.
// NOTE: the clickBullet helper is not reused here because it locates the thought via getEditable, whose XPath
// matches on direct text nodes (contains(text(), …)). A fully-bold thought's text is wrapped in <b>, so the
// editable div has no direct text node and getEditable finds nothing. waitForEditable matches on innerHTML, so
// the bullet is looked up from the <b>One</b> editable handle instead.
const editableOne = await waitForEditable('<b>One</b>')
const bulletNode = await page.evaluateHandle(editableNode => {
if (!editableNode) throw new Error('Node handle does not contain a valid Element')
const thoughtContainer = editableNode.closest('[aria-label="thought-container"]')
if (!thoughtContainer) throw new Error('Thought container not found')
const bullet = thoughtContainer.querySelector('[aria-label="bullet"]')
if (!bullet) throw new Error('Bullet not found in thought container')
return bullet
}, editableOne)
// @ts-expect-error - https://github.com/puppeteer/puppeteer/issues/8852
await bulletNode.asElement()?.click()

// wait for the caret to settle at the start of the bold thought so the command state has updated
await page.waitForFunction(() => (window.getSelection()?.focusOffset ?? -1) === 0)

// the Bold button should reflect the thought's bold formatting rather than flickering back to inactive
expect(await isBoldButtonActive()).toBe(true)

Check failure on line 106 in src/e2e/puppeteer/__tests__/format.ts

View workflow job for this annotation

GitHub Actions / TDD — Puppeteer tests

[puppeteer-e2e] src/e2e/puppeteer/__tests__/format.ts > Bold button stays active when the cursor is moved to a fully-bold thought via its bullet (#3912)

AssertionError: expected false to be true // Object.is equality - Expected + Received - true + false ❯ src/e2e/puppeteer/__tests__/format.ts:106:38
})
Loading