-
-
Notifications
You must be signed in to change notification settings - Fork 128
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: depth issues in BulletedList #347
fix: depth issues in BulletedList #347
Conversation
@gloaysa is attempting to deploy a commit to the dargo's projects Team on Vercel. A member of the Team first needs to authorize it. |
This PR only addresses BulletedList plugin. I will address NumberedList once this one is merged. Things I don't like:
|
I read this is planned for the next release, but I’d appreciate a code review before, please @Darginec05 |
@gloaysa yes, of course, I totally agree with you. P.S. Sorry for the late reply, I was very busy at my main job :D |
|
||
type ListElement = HTMLUListElement | HTMLOListElement | Element | ||
|
||
export function deserializeListNodes(editor: YooEditor, listElement: ListElement): Descendant[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I'll explain first how List blocks works.
By design, each LI
element is essentially separate block. And each block contains only one Slate element:
- Block
TodoList
contains only thetodo-list
- Block
NumberedList
contains only thenumbered-list
- Block
BulletedList
contains only thebulleted-list
What does it mean?
This means that our general function should return non-nested Slate elements, it should return an array of blocks with the correct depth
in the meta
field of the block, because in our case it is the depth
field that will determine the nesting of blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've written my version of the deserializeListNodes
function, which should give you more insight into what I mean:
type ListHTMLElement = HTMLUListElement | HTMLOListElement | Element;
type DeserializeListBlockOptions = {
depth?: number;
type: 'TodoList' | 'NumberedList' | 'BulletedList';
align: YooptaBlockData['meta']['align'];
};
// recursive function to parse nested lists and return list of Blocks
export function deserializeListNodes(
editor: YooEditor,
listEl: ListHTMLElement,
options: DeserializeListBlockOptions,
): YooptaBlockData[] {
const deserializedListBlocks: YooptaBlockData[] = [];
const depth = typeof options.depth === 'number' ? options.depth : 0;
if (listEl.nodeName === 'UL' || listEl.nodeName === 'OL') {
const listItems = Array.from(listEl.children).filter((el) => el.nodeName === 'LI');
// check if the list is a TodoList or not
const isTodoList = listItems.some((listItem) => {
const hasCheckbox = listItem.querySelector('input[type="checkbox"]');
const textContent = listItem.textContent || '';
const hasBrackets = /\[\s*[xX\s]?\s*\]/.test(textContent);
return hasCheckbox || hasBrackets;
});
if (isTodoList && options.type !== 'TodoList') {
return deserializedListBlocks;
}
if (!isTodoList && options.type === 'TodoList') {
return deserializedListBlocks;
}
listItems.forEach((listItem) => {
const textContent = listItem.textContent || '';
let blockData: YooptaBlockData | null = null;
if (options.type === 'TodoList') {
// check if the list item has a checkbox input or brackets `[]` in the text content
const checkbox = listItem.querySelector('input[type="checkbox"]') as HTMLInputElement | null;
const checked = checkbox ? checkbox.checked : /\[\s*[xX]\s*\]/.test(textContent);
const cleanText = textContent
.replace(/\[\s*[xX\s]?\s*\]/, '')
.replace(/\u00a0/g, ' ')
.trim();
blockData = {
id: generateId(),
type: 'TodoList',
value: [
{
id: generateId(),
type: 'todo-list',
children: [{ text: cleanText }],
props: { nodeType: 'block', checked },
},
],
meta: { order: 0, depth, align: options.align },
};
} else {
const listType = options.type === 'NumberedList' ? 'numbered-list' : 'bulleted-list';
blockData = {
id: generateId(),
type: options.type,
value: [
{
id: generateId(),
type: listType,
children: deserializeTextNodes(editor, listItem.childNodes),
props: { nodeType: 'block' },
},
],
meta: { order: 0, depth, align: options.align },
};
}
if (blockData) {
deserializedListBlocks.push(blockData);
}
const nestedLists = Array.from(listItem.children).filter((el) => el.nodeName === 'UL' || el.nodeName === 'OL');
// if the list item has nested lists, then parse them recursively by increasing the depth
if (nestedLists.length > 0) {
nestedLists.forEach((nestedList) => {
const nestedBlocks = deserializeListNodes(editor, nestedList as ListHTMLElement, {
...options,
depth: depth + 1,
});
deserializedListBlocks.push(...nestedBlocks);
});
}
});
}
return deserializedListBlocks;
}
Usage in plugins:
// For example BulletedList
deserialize: {
nodeNames: ['UL'],
parse(el, editor) {
console.log('BulletedList deserialize', el.nodeName);
if (el.nodeName === 'UL') {
const align = (el.getAttribute('data-meta-align') || 'left') as YooptaBlockData['meta']['align'];
const depth = parseInt(el.getAttribute('data-meta-depth') || '0', 10);
const deserializedList = deserializeListNodes(editor, el, { type: 'BulletedList', depth, align });
if (deserializedList.length > 0) {
return deserializedList;
}
}
},
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will work on this during the weekend, thanks!
Description
BulletedList plugin was not ready to handle nested ul/ol elements inside of it.
With this change, a new function that specialises in deserialising list elements is created. We parse each li element found inside a ul/ol element recursively, creating an array of descendants of type list-item for each li found.
Fixes #345
Type of change
Please tick the relevant option.
Checklist: