Open JoshuaDoshua opened 8 years ago
I would also really like to have @jhchen’s thoughts on this. The Cloning Medium with Parchment guide states the following:
[…] While Inline blots can be nested, Block blots cannot. Instead of wrapping, Block blots replace one another when applied to the same text range.
I assume due the lack of support for Block blots nesting, nested lists (as well as nested blockquotes) cannot currently be implemented even with custom Parchment blots, is that correct?
Here's what puzzles me - List actually inherits from Container, rather than from Block - I assume that's how Lists can contain ListItems.
I have tried and tried to create a CustomList that includes CustomList in its allowedChildren, but I haven't been able to get it to work :( In other words, I want this:
<ul class="nugget" nuggetid="1">
<li>An item in the outer list</li>
<ul class="nugget" nuggetid="2">
<li>An item in the inner list</li>
</ul>
</ul>
But no matter how hard I try, Quill ends up modifying that to:
<ul class="nugget" nuggetid="1">
<li>An item in the outer list</li>
</ul>
<ul class="nugget" nuggetid="1">
<li>An item in the inner list</li>
</ul>
[Note that it also mangles the second nuggetid
, replacing "2" with "1".]
For reference, this is the javascript behind that:
'use strict';
import Quill from 'quill';
let Parchment = Quill.import ('parchment');
let List = Parchment.query('list');
let NuggetidAttributor = new Parchment.Attributor.Attribute('nuggetid', 'nuggetid', {
});
class NuggetList extends List {
format(name, value) {
this.domNode.setAttribute('nuggetid', value);
}
}
NuggetList.tagName = 'UL';
NuggetList.className = 'nugget';
NuggetList.allowedChildren.push(NuggetList);
Quill.register(NuggetidAttributor);
Quill.register(NuggetList, true);
If anybody has got this to work, I would love to hear from them.
@rafbm Yes the Blocks are a challenge but using Containers might be a solution but I have not thought about this rigourously.
@JoshuaDoshua By export do you mean paste into other applications?
@jhchen Yessir. Essentially just using the innerHTML
to save the editor content in a way that it can be used elsewhere independent of the indented
class.
Containers does seem promising. But this is definitely a complicated topic. Indenting needs to be aware of above list elements. An interesting note though: The core currently recognizes if you try to apply a list to a new line directly after a list. i.e. create an ordered list with 3 items, return to a new line breaking out of the list, enter some text and apply a list to that line appends it to the previous OL (now containing 4 items)
I also think the output of nested lists is less than ideal, but if we could have a css file with listing styles to make lists look the same as in the editor, maybe it would help initially.
Is there some news about this enhancement?
Using pseudo classes to accomplish nested lists prevents using Quill as a composer for anything that will be sent in email (as email clients do not allow the use of pseudo classes).
@parterburn This is exactly my problem as well, forcing me to pre-parse content and adding actual numbering, which is very inconvenient
@AnrichVS What is your strategy for getting the code back into the editor? Rewriting back to the ql-indent class structure?
I'm currently exploring this, as like you and @parterburn, we use the output in html emails and other places. CSS isn't really a solution. I'm considering writing up a parser that translates the ql-indent li tags into nested uls/ols, etc. Unfortunately, it looks like I'd also have to write the reverse, as pasting nested lists into the editor gives me a flat list every time. So I'd have to rewrite the correct nest back into ql-indented code.
Not something I'm really looking forward to, as it addresses a bit of an edge case, but does look noticeably wrong when a nested ol with numbers and letters gets flattened into one long ol.
We ended up preventing ordered lists from indenting more than once and set the margin-left on unordered lists according the different indent-classes using a premailer stylesheet (https://rubygems.org/gems/premailer-rails).
Haha, pretty much what I'm in the process of doing now. Except I think we are nixing the ordered lists instead of trying to limit them to the parent level. Thanks for the feedback!
Here's our code, if it's helpful:
var bindings = {
"indent": {
key: "tab",
format: ["list"],
handler: function(range, context) {
if(context.format.list == "ordered") {
return false;
} else {
if (context.collapsed && context.offset !== 0) return true;
this.quill.format('indent', '+1', Quill.sources.USER);
}
}
}
};
var emailBodyQuill = new Quill($(".email-body-quill"), {
theme: 'snow',
modules: {
keyboard: {
bindings: bindings
},
toolbar: {
container: [
[{ 'size': ['small', false, 'large', 'huge'] }],
['bold', 'italic', 'underline', 'link'],
[{ 'list': 'ordered' }, { 'list': 'bullet' }, 'blockquote'],
['clean', 'image']
]
}
}
});
I've just come across this problem too.
I'm trying to create PDFs of the editor contents using mPDF, which doesn't support the CSS needed on li's to do it the current Quill.js way. This means that all nested lists have the same level of indentation as the parent, and the numbering on ordered lists is not reset for nested lists (i.e. the numbering continues sequentially for every li in the group of lists).
mPDF does allow setting these on ul's/ol's, so if the indented lists were wrapped in ol or ul, then all would work properly.
Instead I need to write a parser to manually identify and wrap each level (a job I think I'll put off until tomorrow!).
@arist0tl3 I had to put this on hold for quite some time as you can see. My main problem is that I send emails containing Quill created content. On the web side of things all is well, but for emails I use premailer-rails to convert all CSS to inline (to support as many mail clients as possible). And you can't have pseudo selectors (::before
in this case for list numbering) inline.
Thus my plan was, and probably still is, to parse the mails with an interceptor and add actual numbers into the HTML. The indentation CSS works fine inline so I don't have a problem with that, it's just the numbering.
I guess if I don't want multiple levels of numbering (1.1, 1.1.1 etc.), converting the <li>
to properly nested <ol>
and <li>
will also work, but then I'll have to do the same thing on the editor side of things.
I was really hoping some progress has been made on this since my last struggle...
@AnrichVS Agreed. We still haven't figured out a way to handle the numbering in a consistent manner.
Currently, we are just restricting the usage of lists to a single level. Not a great user experience, but we still prefer it to nested lists with broken styles and numbering in our emails.
I toyed with the idea of hooking into the indent code above to write the structure of the current node into an html attribute that could be parsed later to re-write the innerHTML and css, but just couldn't justify putting too much time towards it. I do think that leveraging the indent function to add some useful data to the element is a viable strategy, but again, just can't justify the cost/benefit right now.
Well if I paste from Google Docs to Quill, pretty much everything in a "normal and simple" document is conserved, except the nesting of lists, as requested in this old feature request: "Add support for nested lists/bullets and indents #118" dating back to May 2014. It's right now the only lack of feature that prevents me from using Quill in production.
A list element is relative to a <ul>
or <ol>
element and therefore, can't be represented in a different way than the original html: relatively to the parent element. Lists and list elements can contain each other, just like every DOM element they are nodes, and nodes can contain each other.
The actual fix of using "ql-indent" class isn't fixing anything but the visual. But html is more than a visual, have you every though about how blind people see your documents? You can't remove nesting, it's removing meaning, so it can't be allowed.
I'm fairly new to Quill. But I saw there are a couple tickets that point here, some with discussion whether this is a personal preference or a technical requirement. My statements below support technical requirement and present supporting W3 materials.
In Ontario, a province in Canada, there is legislation for all public institutions and private companies with 50+ employees to have websites and web products be WCAG 2.0 level A compliant today, and by 2021 level AA compliant. There are many other jurisdictions around the world with similar laws and deadlines Eg. US DoJ section 508 law.
Jurisdiction laws aside, we should want to remove barriers to participation online for screenreader and keyboard navigation users, and nested lists are interpreted and navigated differently. eg. skip list or sublist/next.
As @thomasgodart pointed out, in v1.3.5 of Quill, ql-indent-* classes do not provide the content structure needed for accessibility users.
That said, here are technical references by W3 about nested lists:
The following is the code I came up with the convert the quill style lists into "proper" ol/ul nested lists.
Note that this is very hacky code, and will be brittle if quill changes its html rendering. It works on the HTML that Quill currently renders in most browsers (grabbed by .innerHTML or similar). It (obviously) won't work on documents stored as deltas, and may fail if any other processing has altered the structure of the HTML. I use this only when rendering to another format (pdf via MPDF, or non-interactive HTML), storing the unchanged quill document as normal for future editing.
That said, it seem reliable in my use case, an electron app. It uses jquery on a hidden div (to map the html to a dom), you can probably get it to work in node using jsdom or similar. It works on ul, li and mixed combinations. It deals with edge cases like starting a list indented beyond level 0.
I hope this helps someone.
//load a hidden div with the HTML from Quill
$('#manipulator').html(html);
$.each(['ul','ol'], function(index, type) { // works on both types of list
// define some temporary tags <startol>,<endol>,<startul>,<endul>
// which will be replaced with <ol>,</ol>,<ul>,</ul> at the end
// We use temp tags so that they don't get accidentally selected
// as we progress
var start_tag = '<start'+type+'></start'+type+'>';
var end_tag = '<end'+type+'></end'+type+'>';
// Grab each list, and work on it in turn
$('#manipulator '+type).each(function() {
// account for the fact that the first li might not
// be at level 0
var first_li = $(this).children('li').first();
// parse the level from the class name e.g. ql-indent-2 is level 2
// level 0 has no class name
var first_level = $(first_li).attr('class') || '0';
first_level = first_level.replace( /[^\d]/g, '');
// add an appropriate number of opening tags.
$(first_li).before(start_tag.repeat(first_level));
// we don't need to do this at the end, as the last li's "next"
// element isn't a li and thus will always be given a 0 level
// and so the appropriate number of closing tags will be applied
// now work through each li in this List
$(this).children('li').each(function() {
// get the level for this li as above
var current_level = $(this).attr('class') || '0';
current_level = current_level.replace( /[^\d]/g, '');
// get the next li (or false if we're at the end of the list)
var next_li = $(this).next('li') || false;
if (next_li) {
// get the level of the next li as above
var next_level = $(next_li).attr('class') || '0';
next_level = next_level.replace( /[^\d]/g, '');
// work out whether it is a higher level (+ve number)
// or lower (-ve) or at the same level (0)
var difference = next_level - current_level;
// we only need to add tags if the level is changing
if (difference) {
if (difference > 0) {
// if it's a higher level, add an appropriate number
// of opening tags before it
$(next_li).before(start_tag.repeat(difference));
};
if (difference < 0) {
// if it's a lower level, add closing tags instead
difference = 0 - difference; // get abs value
$(this).after(end_tag.repeat(difference));
};
}
}
})
});
})
// grab the html as a string
var new_html = $('#manipulator').html();
// and replace the temp tags with the correct ol/ul ones.
new_html = common.replace_all(new_html, '<startul></startul>', '<ul>');
new_html = common.replace_all(new_html, '<endul></endul>', '</ul>');
new_html = common.replace_all(new_html, '<startol></startol>', '<ol>');
new_html = common.replace_all(new_html, '<endol></endol>', '</ol>');
// new_html now contains the "correct" html.
@RobAley, thanks so much for your solution, I found it very helpful.
For anyone else who needs this and is not using jQuery, here's my ES6 version of Rob's code with a few minor tweaks (all of Rob's caveats apply to this code as well):
Cheers, --Rick
function getListLevel(el) {
const className = el.className || '0';
return +className.replace(/[^\d]/g, '');
}
function convertLists(richtext) {
const tempEl = window.document.createElement('div');
tempEl.setAttribute('style', 'display: none;');
tempEl.innerHTML = richtext;
['ul','ol'].forEach((type) => {
const startTag = `::start${type}::::/start${type}::`;
const endTag = \`::end${type}::::/end${type}::\`;
// Grab each list, and work on it in turn
Array.from(tempEl.querySelectorAll(type)).forEach((outerListEl) => {
const listChildren = Array.from(outerListEl.children).filter((el) => el.tagName === 'LI');
// Account for the fact that the first li might not be at level 0
const firstLi = listChildren[0];
firstLi.before(startTag.repeat(getListLevel(firstLi)));
// Now work through each li in this list
listChildren.forEach((listEl, index) => {
const currentLiLevel = getListLevel(listEl);
if (index < listChildren.length - 1) {
const difference = getListLevel(listChildren[index + 1]) - currentLiLevel;
// we only need to add tags if the level is changing
if (difference > 0) {
listChildren[index + 1].before(startTag.repeat(difference));
} else if (difference < 0) {
listEl.after(endTag.repeat(-difference));
}
} else {
listEl.after(endTag);
}
});
outerListEl.after(endTag);
});
});
// Get the content in the element and replace the temporary tags with new ones
let newContent = tempEl.innerHTML;
newContent = newContent.replace(/::startul::::\/startul::/g, '<ul>');
newContent = newContent.replace(/::endul::::\/endul::/g, '</ul>');
newContent = newContent.replace(/::startol::::\/startol::/g, '<ol>');
newContent = newContent.replace(/::endol::::\/endol::/g, '</ol>');
tempEl.remove();
return newContent;
}
Here is what is expected when handling lists:
https://codepen.io/jasonrundell/pen/zjZrjq
Right now, Quill's lists are not expected behaviour in terms on DOM markup and this creates issues with accessibility and end users who are making lists and not seeing nested lists.
Right now my workaround is a CSS hack with ql-indent-X which is creating a dependency on the Editor when the markup needs to be decoupled from Quill logic.
Also, I can't hack around the fact that the index of the list items won't be correct as I won't be able to modify publisher content every time they add a new list.
Hi, anyone making progress on this? It's too bad there's this discussion because I strongly believe the output of a WYSIWYG editor should be semantically correct (same goes for soft enters). We use Quill for editing emails, which means the ability to style elements is limited.
I think the 'bug' label should be added to this issue.
This is a matter of 'semantically correctness', but also of WYSIWYG.
Ping @jhchen. Please mark this issue as a bug.
I am new to use quill editor and facing issue while copying nested bullets content from MS word doc. Content loses nested bullets and indentation. Can any one help on this?
I just started adding Quill to a project expecting it was the most used/best editor to use. Then I quickly ran into this issue.
Now given how long this issue has been open I'm wondering if I should just switch to something else before it's too late.
@dantman you are right. I did the same step (switched to another editor) more than a year ago. And this issue shows I was right.
Nothing happening on this issue? This is a bug imo
I don't think anything's happening on any bug. Not sure if the maintainers are too busy with v2 (or otherwise), or the projects been abandoned. See #2460.
Btw a coworker of mine updated the code from @rlansky so the sub lists would be inside their own < li > like the html standard is and also some refactor. I needed that so the html would be correct to be injected to turndown.js service.
function getListLevel(el) { const className = el.className || '0'; return +className.replace(/[^\d]/g, ''); }
function convertLists(richtext) {
const tempEl = window.document.createElement('div');
tempEl.setAttribute('style', 'display: none;');
tempEl.innerHTML = richtext;
const startLi = '::startli::';
const endLi = '::endli::';
['ul','ol'].forEach((type) => {
const startTag = `::start${type}::`;
const endTag = `::end${type}::`;
// Grab each list, and work on it in turn
Array.from(tempEl.querySelectorAll(type)).forEach((outerListEl) => {
const listChildren = Array.from(outerListEl.children).filter((el) => el.tagName === 'LI');
let lastLiLevel = 0;
let currentLiLevel = 0;
let difference = 0;
// Now work through each li in this list
for (let i = 0; i < listChildren.length; i++) {
const currentLi = listChildren[i];
lastLiLevel = currentLiLevel;
currentLiLevel = getListLevel(currentLi);
difference = currentLiLevel - lastLiLevel;
// we only need to add tags if the level is changing
if (difference > 0) {
currentLi.before((startLi + startTag).repeat(difference));
} else if (difference < 0) {
currentLi.before((endTag + endLi).repeat(-difference));
}
if (i === listChildren.length - 1) {
// last li, account for the fact that it might not be at level 0
currentLi.after((endTag + endLi).repeat(currentLiLevel));
}
}
});
});
// Get the content in the element and replace the temporary tags with new ones
let newContent = tempEl.innerHTML;
newContent = newContent.replace(/::startul::/g, '<ul>');
newContent = newContent.replace(/::endul::/g, '</ul>');
newContent = newContent.replace(/::startol::/g, '<ol>');
newContent = newContent.replace(/::endol::/g, '</ol>');
newContent = newContent.replace(/::startli::/g, '<li>');
newContent = newContent.replace(/::endli::/g, '</li>');
tempEl.remove();
return newContent;
}
It seems that Quill works just fine, it gives you the right html and each list element posses its own class eg: ql-indent-4, or ql-indent-1 depending on the level of indentation.
My issue was the following: I was saving the html returned by the onChange handler and displaying it into a different component. Everything was showing fine inside Quill's textarea and my component's as well. However, despite nested lists being rendered properly inside Quill's textarea, they were not in my component. What I ended up doing was importing quill's css into my component or at the root level and that fixed the issue with.
import 'react-quill/dist/quill.core.css'
import 'react-quill/dist/quill.bubble.css'
import 'react-quill/dist/quill.snow.css'
Also make sure that you wrap your custom component where you display the html markup with the "ql-editor" class. If you take a look at the stylesheets the indentation selectors are defined as
".ql-editor .ql-index-1"
Also, I was adding extra indentation with the Tab key.
No, no, no. This is not working fine.
Adding a class to indent bullet points is semantically incorrect. We don't use <p style="display: block; font-size: 2em; margin-top: 0.67em; margin-bottom: 0.67em; margin-left: 0; margin-right: 0; font-weight: bold"></p>
or <p class"heading1"></p>
instead of <h1></h1>
either, do we?
We don't do this because it is incorrect. h1
is not a shortcut for a bigger font size, it tells us something about the structure of the content. You can add a ToC client side, search engines use it, it improves accessibility, etc. There are many more good reasons why one should aim for semantically correct websites.
My application sends HTML to a third party and I'm not even allowed to send additional CSS with it. So right now I can't use this, because the other party won't understand the structure of the lists.
@stephanvierkant Thanks for fighting this fight Stephan :) I totally agree. We're using the editor for HTML in email and are als not able to use CSS to style the lists.
@robin3317 It is not working fine if you want to indent an unordered list under an ordered one. Since quill switches from ol
-> ul
-> ol
, you get this:
1. First numbered bullet
- Indented bullet
1. This should be number 2
oh fuck.. I have to change so much to switch to another editor... ( Why this behaviour is not in the docs!???? It would saved me a lot of time
Workaround: https://gist.github.com/Vovan-VE/46934917f4c39a07705c5012a9b9fba9
Redux ready. See integration.js
@stephanvierkant You are totally right. It makes a lot of sense what you said above. Ideally we would probably want the handler to return us the html markup with the styles already injected rather than with the class names. Thank you for enlightening us. Cheers
@Vovan-VE thanks a lot. Pretty hacky, but with some adjustments to your code we got it working for a while...
I disabled the TAB key event in the Quill editor to support tabbing to other control elements once your focus is in the Quill editor. This also disables creating nested lists in the editor which at least keeps the HTML content from the editor compliant/accurate.
delete quill.getModule('keyboard').bindings["9"];
This appears to still be an issue. integrated quill into my app very heavily but i will switch if this hasn't been fixed properly as Im running into issues as well with export to other things (email, pdf ect).
Anyone have a working module to fix this?
a. Test b. Post c. Test d. Pots
Found an alternative to @rbonomo 's solution for disabling nesting. The binding can be overriden using the options object used to initialize quill with:
{
modules: {
keyboard: {
bindings: {
indent: {
key: 'Tab',
format: ['blockquote', 'indent', 'list'],
handler: (_range: any, _context: any) => {
// nop
}
}
}
}
}
}
My issue was the following: I was saving the html returned by the onChange handler and displaying it into a different component. Everything was showing fine inside Quill's textarea and my component's as well. However, despite nested lists being rendered properly inside Quill's textarea, they were not in my component. What I ended up doing was importing quill's css into my component or at the root level and that fixed the issue with.
import 'react-quill/dist/quill.core.css' import 'react-quill/dist/quill.bubble.css' import 'react-quill/dist/quill.snow.css'
Also make sure that you wrap your custom component where you display the html markup with the "ql-editor" class. If you take a look at the stylesheets the indentation selectors are defined as
".ql-editor .ql-index-1"
Also, I was adding extra indentation with the Tab key.
Thank you for the temp solution! Adding "class=ql-editor" displayed the formatting correctly for me.
Yea this works great in Browers and its simplicity is what got me excited. But when the content goes to our PDF and RTF tools, the lists aren't indented correctly. Ill have to stick with CKEditor but i hate it.. Please implement standard list nesting as its the only blocker for us using it.
Why is quill adding custom classes that are not renderable outside of quill editor unless you copy paste them from quill's css. This is not WYSIWYG
I got a few problems with code updated by @rlansky / @maggask. Nested lists should be placed inside previous "li" element, otherwise we can get empty list item.
Actual:
- 1
- 2
- 2.1
Expected:
- 1
- 2
- 2.1
I have fixed this behavior in my Gist here - https://gist.github.com/Daenero/3442213dc5093dc10f30711edb529729.
And also, as in example from @Vovan-VE, you can also encode HTML nested list to Quill format.
I guess it should help.
Thanks @Daenero for the code, it worked for my use case.
I just fixed a corner case, when lists are indented without parent (see my comment on your gist : https://gist.github.com/Daenero/3442213dc5093dc10f30711edb529729#gistcomment-3737134)
And for those who use this, don't forget to sanitize the input when using quillEncodeIndent
, otherwise there is a high risk of XSS attack.
@RobAley, thanks so much for your solution, I found it very helpful.
For anyone else who needs this and is not using jQuery, here's my ES6 version of Rob's code with a few minor tweaks (all of Rob's caveats apply to this code as well):
* I found that there were some missing tags for closing the lists, so I added some additional close tags at the end of the for-loops. * I was having problems with the < and > symbols in the start_tag and end_tag parameters getting encoding into < and > when I injected them into the document, so I changed those to :: instead. * I'm hiding the temporary element and removing it once I'm done with it.
Cheers, --Rick
function getListLevel(el) { const className = el.className || '0'; return +className.replace(/[^\d]/g, ''); } function convertLists(richtext) { const tempEl = window.document.createElement('div'); tempEl.setAttribute('style', 'display: none;'); tempEl.innerHTML = richtext; ['ul','ol'].forEach((type) => { const startTag = `::start${type}::::/start${type}::`; const endTag = \`::end${type}::::/end${type}::\`; // Grab each list, and work on it in turn Array.from(tempEl.querySelectorAll(type)).forEach((outerListEl) => { const listChildren = Array.from(outerListEl.children).filter((el) => el.tagName === 'LI'); // Account for the fact that the first li might not be at level 0 const firstLi = listChildren[0]; firstLi.before(startTag.repeat(getListLevel(firstLi))); // Now work through each li in this list listChildren.forEach((listEl, index) => { const currentLiLevel = getListLevel(listEl); if (index < listChildren.length - 1) { const difference = getListLevel(listChildren[index + 1]) - currentLiLevel; // we only need to add tags if the level is changing if (difference > 0) { listChildren[index + 1].before(startTag.repeat(difference)); } else if (difference < 0) { listEl.after(endTag.repeat(-difference)); } } else { listEl.after(endTag); } }); outerListEl.after(endTag); }); }); // Get the content in the element and replace the temporary tags with new ones let newContent = tempEl.innerHTML; newContent = newContent.replace(/::startul::::\/startul::/g, '<ul>'); newContent = newContent.replace(/::endul::::\/endul::/g, '</ul>'); newContent = newContent.replace(/::startol::::\/startol::/g, '<ol>'); newContent = newContent.replace(/::endol::::\/endol::/g, '</ol>'); tempEl.remove(); return newContent; }
After this we don't need class ql-indent-1
and ql-indent-2
to list. If class present then style will also set, so I removed :
`... newContent = newContent.replace(/ class="ql-indent-1"/g, '') newContent = newContent.replace(/ class="ql-indent-2"/g, '')
tempEl.remove(); return newContent;`
this is really huge issue for me. I'm implementing several formats of lists and now I realised that my lists does not work as they should. @Daenero Please, how can I apply Your Fix in code?
@JurajKavka @Daenero Please let us know how to implement your fix. This is a serious issue.
@JurajKavka @Daenero what is the status of this ticket?
Indent behavior could be improved.
Indenting a list item should wrap the current list item in a new
<ul><li>
or<ol><li>
and nest it in the closest<li>
tag (or do nothing if you are at the highest level or there is only one item in the list)The current indent button simply adds a class to the
<li>
, which looks great but outputs bad HTML. The generated content would not be exportable to other platforms.v1.0.3
This seems a bit more complicated than a simple fix. If I have time I'll try to get a PR in, but wanted to get the topic open for discussion