fix(TaskItem): use div instead of label element as task item wrapper

[mejo-]

📝 Summary

Fixes problems with more than one block node in task items. label elements have special click behaviour: clicking on them fires a second click on the their first "labelable" descendant.

This resulted in weird bugs when having e.g. a code block or image as second block node in a task item.

• Fixes: Not possible to click into list item which also includes an image #5225

🏁 Checklist

• Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
• Sign-off message is added to all commits

[mejo-]

/backport to stable33

[max-nextcloud]

I think if you have an object that never changes you can get a very precise type annotation with Object.freeze for free:

Suggested change

	const checkboxAttributes: Object.freeze({
	type: 'checkbox',
	class: '',
	contenteditable: false,
	...( node.attrs.checked ? { checked: true } : {} ),
	})

In this case checkboxAttributes.type will not just be a string but the string checkbox and it will be readonly.

[mejo-]

Interesting, I discussed this a bit with Claude and here's it's thoughts:

The argument is partially valid — Object.freeze does cause TypeScript to infer literal types and readonly modifiers. But it's the wrong tool for achieving that goal in TypeScript code.

The idiomatic solution: as const

const checkboxAttributes = {
    type: 'checkbox',
    class: '',
    contenteditable: false,
    ...(node.attrs.checked ? { checked: true } : {}),
} as const

Why as const is better than Object.freeze here:

_	Object.freeze	as const
Literal type inference ('checkbox' not string)	Yes	Yes
readonly properties	Yes	Yes
Runtime overhead	Yes (actual freeze operation)	None (compile-time only)
Idiomatic TypeScript	No	Yes
Enforces immutability at runtime	Yes (throws/silently fails)	No

The key distinction: as const is a compile-time type assertion that achieves exactly the same typing benefits the author was after, with zero runtime cost. Object.freeze is a runtime operation that happens to also affect TypeScript's type inference as a side effect — that's backwards reasoning for a TypeScript codebase.

The fact that the object is never mutated after creation is a TypeScript-design concern, not a runtime concern. as const is the correct way to communicate and enforce that intent in the type system.

One nuance to be aware of: with a conditional spread and as const, TypeScript infers a union-ish type for the resulting object, so checked becomes true (literal) when present — which is actually more precise than the original checked?: true annotation. This is a genuine improvement.

[mejo-]

I refactored it to use as const now and also fixed the tiptap.spec.js test.

[max-nextcloud]

A nice... Did not know about as const. That's inded better.

[max-nextcloud]

I'm wondering if this has downsides in terms of accessibility. I checked but right now that extra click is not used, right? At least clicking the text of a task item does not (un-)check it.

[mejo-]

I'm wondering if this has downsides in terms of accessibility. I checked but right now that extra click is not used, right? At least clicking the text of a task item does not (un-)check it.

Actually I think it's the other way round: accessibility improved as we no longer have label elements that lack a sibling element that they're the label for. The double click was actually a bug in my understanding.

[codecov]

Codecov Report

❌ Patch coverage is 72.22222% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/nodes/TaskItem.ts	72.22%	3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!