Skip to content

Commit

Permalink
refactor(portable-text-editor): automatically resolve some validation…
Browse files Browse the repository at this point in the history
… errors (#6705)

* fix(portable-text-editor): normalize missing .markDefs on text block schema type

* refactor(portable-text-editor): refactor addAnnotation fn

This was too damn messy and contained a bug where sometimes an annotation would not be added.
Fixed the bug.

* refactor(portable-text-editor): don't automatically add missing markDefs to editor value

We will normalize this automatically and update the value when needed. For this to work, the value must reflect the original value

* feat(portable-text-editor): normalize away marks that doesn't exist in the the schema

Note: you need to actively edit the node before it's normalized

* test(portable-text-editor): update tests to test for new normalization of marks and markdefs

* chore(portable-text-editor): use tsx over ts-node

* refactor(portable-text-editor): automatically resolve some validation error

Resolve errors for missing .markDefs, orphapned marks, empty children, missing child keys

* fix(portable-text-editor): allow for missing span marks in value

Don't insert a empty array for missing span node .marks. It will be inserted
through normalization inside withPortableTextMarkModel instead.

* doc(portable-text-editor): update comments

* fix(portable-text-editor): validate missing and empty children

* test(portable-text-editor): update test to test for missing children

* fix(types): fix invalid assertion in isPortableTextTextBlock

As .markDefs is optional, return true when missing

* fix(portable-text-editor): don't validate or normalize missing .markDefs

It should be allowed and is a optional property according to the schema
  • Loading branch information
skogsmaskin committed May 22, 2024
1 parent 922235f commit 6062a9a
Show file tree
Hide file tree
Showing 15 changed files with 454 additions and 256 deletions.
3 changes: 2 additions & 1 deletion packages/@sanity/portable-text-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"build": "pkg-utils build --strict --check --clean",
"check:types": "tsc --project tsconfig.lib.json",
"clean": "rimraf lib",
"dev": "cd ./e2e-tests/ && ts-node serve",
"dev": "cd ./e2e-tests/ && tsx serve",
"lint": "eslint .",
"prepublishOnly": "turbo run build",
"prettier": "prettier --write './**/*.{ts,tsx,js,css,html}'",
Expand Down Expand Up @@ -97,6 +97,7 @@
"rimraf": "^3.0.2",
"rxjs": "^7.8.1",
"styled-components": "^6.1.11",
"tsx": "^4.10.3",
"vite": "^4.5.3"
},
"peerDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,19 +330,12 @@ describe('initialization', () => {
await waitFor(() => {
if (editorRef.current) {
expect(onChange).toHaveBeenCalledWith({
type: 'invalidValue',
value: initialValue,
resolution: {
action: 'Remove invalid marks',
description:
"Block with _key 'abc' contains marks (invalid) not supported by the current content model.",
item: {
_key: 'abc',
_type: 'myTestBlockType',
children: [{_key: 'def', _type: 'span', marks: ['invalid'], text: 'Test'}],
markDefs: [],
},
patches: [
{path: [{_key: 'abc'}, 'children', {_key: 'def'}, 'marks'], type: 'set', value: []},
],
i18n: {
action: 'inputs.portable-text.invalid-value.orphaned-marks.action',
description: 'inputs.portable-text.invalid-value.orphaned-marks.description',
Expand All @@ -351,26 +344,40 @@ describe('initialization', () => {
orphanedMarks: ['invalid'],
},
},
},
type: 'invalidValue',
value: [
{
_key: '123',
_type: 'myTestBlockType',
children: [{_key: '567', _type: 'span', marks: [], text: 'Hello'}],
markDefs: [],
},
{
item: {
_key: 'abc',
_type: 'myTestBlockType',
children: [{_key: 'def', _type: 'span', marks: ['invalid'], text: 'Test'}],
children: [
{
_key: 'def',
_type: 'span',
marks: ['invalid'],
text: 'Test',
},
],
markDefs: [],
},
],
patches: [
{
path: [
{
_key: 'abc',
},
'children',
{
_key: 'def',
},
'marks',
],
type: 'set',
value: [],
},
],
},
})
expect(onChange).not.toHaveBeenCalledWith({type: 'value', value: initialValue})
expect(onChange).toHaveBeenCalledWith({type: 'ready'})
}
})
expect(onChange).not.toHaveBeenCalledWith({type: 'value', value: initialValue})
expect(onChange).toHaveBeenCalledWith({type: 'ready'})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import {describe, expect, it, jest} from '@jest/globals'
import {render, waitFor} from '@testing-library/react'
import {createRef, type RefObject} from 'react'

import {PortableTextEditor} from '../PortableTextEditor'
import {PortableTextEditorTester, schemaType} from './PortableTextEditorTester'

describe('when PTE would display warnings, instead it self solves', () => {
it('when child at index is missing required _key in block with _key', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'abc',
_type: 'myTestBlockType',
children: [
{
_type: 'span',
marks: [],
text: 'Hello with a new key',
},
],
markDefs: [],
style: 'normal',
},
]

const onChange = jest.fn()
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
await waitFor(() => {
expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue})
expect(onChange).toHaveBeenCalledWith({type: 'ready'})
})
await waitFor(() => {
if (editorRef.current) {
expect(PortableTextEditor.getValue(editorRef.current)).toEqual([
{
_key: 'abc',
_type: 'myTestBlockType',
children: [
{
_key: '4',
_type: 'span',
text: 'Hello with a new key',
marks: [],
},
],
markDefs: [],
style: 'normal',
},
])
}
})
})

it('allows missing .markDefs', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'abc',
_type: 'myTestBlockType',
children: [
{
_key: 'def',
_type: 'span',
marks: [],
text: 'No markDefs',
},
],
style: 'normal',
},
]

const onChange = jest.fn()
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
await waitFor(() => {
expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue})
expect(onChange).toHaveBeenCalledWith({type: 'ready'})
})
await waitFor(() => {
if (editorRef.current) {
PortableTextEditor.focus(editorRef.current)
expect(PortableTextEditor.getValue(editorRef.current)).toEqual([
{
_key: 'abc',
_type: 'myTestBlockType',
children: [
{
_key: 'def',
_type: 'span',
text: 'No markDefs',
marks: [],
},
],
style: 'normal',
},
])
}
})
})

it('adds missing .children', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'abc',
_type: 'myTestBlockType',
style: 'normal',
markDefs: [],
},
{
_key: 'def',
_type: 'myTestBlockType',
style: 'normal',
children: [],
markDefs: [],
},
]

const onChange = jest.fn()
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
await waitFor(() => {
expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue})
expect(onChange).toHaveBeenCalledWith({type: 'ready'})
})
await waitFor(() => {
if (editorRef.current) {
PortableTextEditor.focus(editorRef.current)
expect(PortableTextEditor.getValue(editorRef.current)).toEqual([
{
_key: 'abc',
_type: 'myTestBlockType',
children: [
{
_key: '5',
_type: 'span',
text: '',
marks: [],
},
],
markDefs: [],
style: 'normal',
},
{
_key: 'def',
_type: 'myTestBlockType',
children: [
{
_key: '6',
_type: 'span',
text: '',
marks: [],
},
],
markDefs: [],
style: 'normal',
},
])
}
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,24 @@ export function useSyncValue(
if (hasChanges && isValid) {
const validationValue = [value[currentBlockIndex]]
const validation = validateValue(validationValue, schemaTypes, keyGenerator)
if (validation.valid) {
// Resolve validations that can be resolved automatically, without involving the user (but only if the value was changed)
if (
!validation.valid &&
validation.resolution?.autoResolve &&
validation.resolution?.patches.length > 0
) {
// Only apply auto resolution if the value has been populated before and is different from the last one.
if (!readOnly && previousValue.current && previousValue.current !== value) {
// Give a console warning about the fact that it did an auto resolution
console.warn(
`${validation.resolution.action} for block with _key '${validationValue[0]._key}'. ${validation.resolution?.description}`,
)
validation.resolution.patches.forEach((patch) => {
change$.next({type: 'patch', patch})
})
}
}
if (validation.valid || validation.resolution?.autoResolve) {
if (oldBlock._key === currentBlock._key) {
if (debug.enabled) debug('Updating block', oldBlock, currentBlock)
_updateBlock(slateEditor, currentBlock, oldBlock, currentBlockIndex)
Expand All @@ -168,7 +185,7 @@ export function useSyncValue(
'Validating and inserting new block in the end of the value',
currentBlock,
)
if (validation.valid) {
if (validation.valid || validation.resolution?.autoResolve) {
withPreserveKeys(slateEditor, () => {
Transforms.insertNodes(slateEditor, currentBlock, {
at: [currentBlockIndex],
Expand Down Expand Up @@ -255,6 +272,7 @@ function _replaceBlock(
withPreserveKeys(slateEditor, () => {
Transforms.insertNodes(slateEditor, currentBlock, {at: [currentBlockIndex]})
})
slateEditor.onChange()
if (selectionFocusOnBlock) {
Transforms.select(slateEditor, currentSelection)
}
Expand Down
Loading

0 comments on commit 6062a9a

Please sign in to comment.