Skip to content
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.

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

fix(pte): tools are active only when all blocks use the tool #6524

Merged
merged 39 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
c8c91c1
fix(pte): show toolbar as active only if all blocks have tool active
jordanl17 Apr 23, 2024
038faba
fix(pte): show toolbar as active only if all blocks have tool active
jordanl17 Apr 24, 2024
03b3ad8
Merge branch 'next' into EDX-801-2
jordanl17 Apr 29, 2024
ac6ed9c
fix(pte): correct toggling of marks when part mark is selected
jordanl17 Apr 29, 2024
a44579f
fix(core): change scrollintoview block to be nearest (#6328)
ninaandal Apr 29, 2024
2dac130
fix(core): set scroll boundary to nearest (#6310)
ninaandal Apr 29, 2024
f76741b
fix(@sanity): issue where hidden unicode characters were bloating doc…
RitaDias Apr 29, 2024
7a107e5
chore: add codeowners to block-tools
skogsmaskin Apr 29, 2024
7c3b865
chore(deps): dedupe pnpm-lock.yaml (#6508)
ecospark[bot] Apr 29, 2024
3b683fe
fix(pte): tidying implementation
jordanl17 Apr 29, 2024
fec2fbc
fix(@sanity): issue where hidden unicode characters were bloating doc…
RitaDias Apr 29, 2024
29eb806
chore(deps): dedupe pnpm-lock.yaml (#6508)
ecospark[bot] Apr 29, 2024
09e84a0
chore(deps): update dependency @sanity/pkg-utils to v6.8.8 (#6509)
renovate[bot] Apr 29, 2024
caaca42
feat: use prefersLatestPublished parameter in DocumentPaneProvider (#…
cngonzalez Apr 29, 2024
3332ae4
fix(@sanity): issue where hidden unicode characters were bloating doc…
RitaDias Apr 29, 2024
b9ac7cb
chore(deps): dedupe pnpm-lock.yaml (#6508)
ecospark[bot] Apr 29, 2024
81e4d01
chore(deps): update dependency @sanity/pkg-utils to v6.8.8 (#6509)
renovate[bot] Apr 29, 2024
a5fbc05
Merge branch 'next' into EDX-801-2
jordanl17 Apr 29, 2024
10a080e
fix(pte): simplifying activeAnnotations logic
jordanl17 Apr 29, 2024
2f0e9e7
fix(pte): new isAnnotationActive static method
jordanl17 Apr 29, 2024
0b25120
fix(pte): retoring activeAnnotations method in editor
jordanl17 Apr 29, 2024
d3c0020
fix(pte): retoring activeAnnotations method in editor
jordanl17 Apr 29, 2024
6a2554b
Merge branch 'next' into EDX-801-2
jordanl17 Apr 30, 2024
07617b3
fix(pte): fixing issue with multiple annotations in a single block
jordanl17 Apr 30, 2024
fc5a2b6
fix(pte): fixing issue with multiple annotations in a single block
jordanl17 Apr 30, 2024
569c9e9
Merge branch 'next' into EDX-801-2
jordanl17 Apr 30, 2024
b1f3490
fix(pte): fixing typing issue with PortableTextSpan
jordanl17 Apr 30, 2024
b35f437
fix(pte): fixing failing test since mark toggling has changed
jordanl17 Apr 30, 2024
f47844a
fix(pte): improving usage of isTextBlock for createWithPortableTextLists
jordanl17 Apr 30, 2024
261b106
fix(portable-text-editor): fix issue where decoration would not targe…
skogsmaskin Apr 30, 2024
363f339
Merge branch 'next' into EDX-801-2
jordanl17 Apr 30, 2024
aee350a
Merge branch 'next' into EDX-801-2
jordanl17 May 1, 2024
d46a1a2
fix(pte): remove unused function in createWithPortableTextMarkModel
jordanl17 May 1, 2024
e30e42b
exploration
jordanl17 May 2, 2024
a2df20a
Merge branch 'next' into EDX-801-2
jordanl17 May 2, 2024
c045eb7
fix(pte): reverting incorrect merge
jordanl17 May 2, 2024
f57db72
fix(pte): testing new logic for tools in PTE
jordanl17 May 2, 2024
d85ba5a
Merge branch 'next' into EDX-801-2
jordanl17 May 2, 2024
bb5d251
fix(pte): reorg test cases
jordanl17 May 2, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ export class PortableTextEditor extends Component<PortableTextEditorProps> {
static activeAnnotations = (editor: PortableTextEditor): PortableTextObject[] => {
return editor && editor.editable ? editor.editable.activeAnnotations() : []
}
static isAnnotationActive = (
editor: PortableTextEditor,
annotationType: PortableTextObject['_type'],
): boolean => {
return editor && editor.editable ? editor.editable.isAnnotationActive(annotationType) : false
}
static addAnnotation = (
editor: PortableTextEditor,
type: ObjectSchemaType,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import {describe, expect, it, jest} from '@jest/globals'
import {render, waitFor} from '@testing-library/react'
import {createRef, type RefObject} from 'react'

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

describe('plugin:withPortableTextLists', () => {
it('should return active list styles that cover the whole selection', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'a',
_type: 'myTestBlockType',
children: [
{
_key: 'a1',
_type: 'span',
marks: [],
text: '12',
},
],
markDefs: [],
style: 'normal',
},
{
_key: 'b',
_type: 'myTestBlockType',
children: [
{
_key: '2',
_type: 'span',
marks: [],
text: '34',
level: 1,
listItem: 'bullet',
},
],
markDefs: [],
style: 'normal',
},
]
const onChange = jest.fn()
await waitFor(() => {
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
})
const editor = editorRef.current!
expect(editor).toBeDefined()
await waitFor(() => {
PortableTextEditor.focus(editor)
PortableTextEditor.select(editor, {
focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0},
anchor: {path: [{_key: '2'}, 'children', {_key: '2'}], offset: 2},
})
expect(PortableTextEditor.hasListStyle(editor, 'bullet')).toBe(false)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -360,23 +360,25 @@ describe('plugin:withPortableTextMarksModel', () => {
})
PortableTextEditor.toggleMark(editor, 'bold')
expect(PortableTextEditor.getValue(editor)).toMatchInlineSnapshot(`
Array [
Object {
"_key": "a",
"_type": "myTestBlockType",
"children": Array [
Object {
"_key": "a1",
"_type": "span",
"marks": Array [],
"text": "1234",
},
],
"markDefs": Array [],
"style": "normal",
},
]
`)
Array [
Object {
"_key": "a",
"_type": "myTestBlockType",
"children": Array [
Object {
"_key": "a1",
"_type": "span",
"marks": Array [
"bold",
],
"text": "1234",
},
],
"markDefs": Array [],
"style": "normal",
},
]
`)
}
})
})
Expand Down Expand Up @@ -765,7 +767,110 @@ describe('plugin:withPortableTextMarksModel', () => {
expect(currentSelectionObject === nextSelectionObject).toBe(false)
expect(onChange).toHaveBeenCalledWith({type: 'selection', selection: nextSelectionObject})
})

it('should return active marks that cover the whole selection', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'a',
_type: 'myTestBlockType',
children: [
{
_key: 'a1',
_type: 'span',
marks: ['bold'],
text: '12',
},
{
_key: '2',
_type: 'span',
marks: [],
text: '34',
},
],
markDefs: [{_key: 'bold', _type: 'strong'}],
style: 'normal',
},
]
const onChange = jest.fn()
await waitFor(() => {
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
})
const editor = editorRef.current!
expect(editor).toBeDefined()
await waitFor(() => {
PortableTextEditor.focus(editor)
PortableTextEditor.select(editor, {
focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0},
anchor: {path: [{_key: 'a'}, 'children', {_key: '2'}], offset: 2},
})
expect(PortableTextEditor.isMarkActive(editor, 'bold')).toBe(false)
PortableTextEditor.toggleMark(editor, 'bold')
expect(PortableTextEditor.isMarkActive(editor, 'bold')).toBe(true)
})
})

it('should return active annotation types that cover the whole selection', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
const initialValue = [
{
_key: 'a',
_type: 'myTestBlockType',
children: [
{
_key: 'a1',
_type: 'span',
marks: ['bab319ad3a9d'],
text: '12',
},
{
_key: '2',
_type: 'span',
marks: [],
text: '34',
},
],
markDefs: [
{
_key: 'bab319ad3a9d',
_type: 'link',
href: 'http://www.123.com',
},
],
style: 'normal',
},
]
const onChange = jest.fn()
await waitFor(() => {
render(
<PortableTextEditorTester
onChange={onChange}
ref={editorRef}
schemaType={schemaType}
value={initialValue}
/>,
)
})
const editor = editorRef.current!
expect(editor).toBeDefined()
await waitFor(() => {
PortableTextEditor.focus(editor)
PortableTextEditor.select(editor, {
focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0},
anchor: {path: [{_key: 'a'}, 'children', {_key: '2'}], offset: 2},
})
expect(PortableTextEditor.isAnnotationActive(editor, 'link')).toBe(false)
})
})
})

describe('removing annotations', () => {
it('removes the markDefs if the annotation is no longer in use', async () => {
const editorRef: RefObject<PortableTextEditor> = createRef()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
isPortableTextSpan,
type ObjectSchemaType,
type Path,
type PortableTextBlock,
Expand Down Expand Up @@ -295,6 +296,47 @@ export function createWithEditableAPI(
return []
}
},
isAnnotationActive: (annotationType: PortableTextObject['_type']): boolean => {
if (!editor.selection || editor.selection.focus.path.length < 2) {
return false
}

try {
const spans = [
...Editor.nodes(editor, {
at: editor.selection,
match: (node) => Text.isText(node),
}),
]

if (
spans.some(
([span]) => !isPortableTextSpan(span) || !span.marks || span.marks?.length === 0,
)
)
return false

const selectionMarkDefs = spans.reduce((accMarkDefs, [, path]) => {
const [block] = Editor.node(editor, path, {depth: 1})
if (editor.isTextBlock(block) && block.markDefs) {
return [...accMarkDefs, ...block.markDefs]
}
return accMarkDefs
}, [] as PortableTextObject[])

return spans.every(([span]) => {
if (!isPortableTextSpan(span)) return false

const spanMarkDefs = span.marks?.map(
(markKey) => selectionMarkDefs.find((def) => def?._key === markKey)?._type,
)

return spanMarkDefs?.includes(annotationType)
})
} catch (err) {
return false
}
},
addAnnotation: (
type: ObjectSchemaType,
value?: {[prop: string]: unknown},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,14 @@ export function createWithPortableTextLists(types: PortableTextMemberSchemaTypes
const selectedBlocks = [
...Editor.nodes(editor, {
at: editor.selection,
match: (node) => editor.isListBlock(node) && node.listItem === listStyle,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the face of it, this seems identical, but match essentially filters all blocks in the selection, which then conceals whether any of these blocks are not of a list type

match: (node) => editor.isTextBlock(node),
}),
]

if (selectedBlocks.length > 0) {
return true
return selectedBlocks.every(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to return every here or do we just care that there is > 0 blocks that meet this condition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do care that every block has the list style. For example if the selection was on the following:

First block

  • List second block

Third block

we'd have 3 selectedBlocks, but only 1 would have bullet list style, so we'd want to return false

([node]) => editor.isListBlock(node) && node.listItem === listStyle,
)
}
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,9 @@
*
*/

import {flatten, isEqual, uniq} from 'lodash'
import {isEqual, uniq} from 'lodash'
import {type Subject} from 'rxjs'
import {
type Descendant,
Editor,
Element,
type NodeEntry,
Path,
Range,
Text,
Transforms,
} from 'slate'
import {type Descendant, Editor, Element, Path, Range, Text, Transforms} from 'slate'

import {
type EditorChange,
Expand Down Expand Up @@ -250,9 +241,8 @@ export function createWithPortableTextMarkModel(
const splitTextNodes = [
...Editor.nodes(editor, {at: editor.selection, match: Text.isText}),
]
const shouldRemoveMark = flatten(
splitTextNodes.map((item) => item[0]).map((node) => node.marks),
).includes(mark)
const shouldRemoveMark = splitTextNodes.every((node) => node[0].marks?.includes(mark))

if (shouldRemoveMark) {
editor.removeMark(mark)
return editor
Expand Down Expand Up @@ -345,19 +335,24 @@ export function createWithPortableTextMarkModel(
if (!editor.selection) {
return false
}
let existingMarks =

const selectedNodes = Array.from(
Editor.nodes(editor, {match: Text.isText, at: editor.selection}),
)

if (Range.isExpanded(editor.selection)) {
return selectedNodes.every((n) => {
const [node] = n

return node.marks?.includes(mark)
})
}

return (
{
...(Editor.marks(editor) || {}),
}.marks || []
if (Range.isExpanded(editor.selection)) {
Array.from(Editor.nodes(editor, {match: Text.isText, at: editor.selection})).forEach(
(n) => {
const [node] = n as NodeEntry<Text>
existingMarks = uniq([...existingMarks, ...((node.marks as string[]) || [])])
},
)
}
return existingMarks.includes(mark)
).includes(mark)
}

// Custom editor function to toggle a mark
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface EditableAPIDeleteOptions {
/** @beta */
export interface EditableAPI {
activeAnnotations: () => PortableTextObject[]
isAnnotationActive: (annotationType: PortableTextObject['_type']) => boolean
addAnnotation: (
type: ObjectSchemaType,
value?: {[prop: string]: unknown},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,10 @@ export function useActiveActionKeys({
return useUnique(
useMemo(
() => {
const activeAnnotationKeys = PortableTextEditor.activeAnnotations(editor).map(
(a) => a._type,
)

return actions
.filter((a) => {
if (a.type === 'annotation') {
return activeAnnotationKeys.includes(a.key)
return PortableTextEditor.isAnnotationActive(editor, a.key)
}

if (a.type === 'listStyle') {
Expand Down
Loading