quill icon indicating copy to clipboard operation
quill copied to clipboard

List nesting/indent HTML is not semantic

Open JoshuaDoshua opened this issue 9 years ago • 70 comments

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

JoshuaDoshua avatar Sep 16 '16 00:09 JoshuaDoshua

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?

rafbm avatar Sep 22 '16 15:09 rafbm

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.

gregdetre avatar Sep 22 '16 16:09 gregdetre

@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 avatar Sep 22 '16 18:09 jhchen

@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)

JoshuaDoshua avatar Sep 22 '16 19:09 JoshuaDoshua

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.

luisrudge avatar Feb 12 '17 01:02 luisrudge

Is there some news about this enhancement?

m00nk avatar Jun 07 '17 11:06 m00nk

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 avatar Jun 09 '17 21:06 parterburn

@parterburn This is exactly my problem as well, forcing me to pre-parse content and adding actual numbering, which is very inconvenient

AnrichVS avatar Jul 13 '17 08:07 AnrichVS

@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.

arist0tl3 avatar Jul 30 '17 01:07 arist0tl3

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).

parterburn avatar Jul 30 '17 02:07 parterburn

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!

arist0tl3 avatar Jul 30 '17 02:07 arist0tl3

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']
      ]
    }
  }
});

parterburn avatar Jul 30 '17 02:07 parterburn

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!).

RobAley avatar Oct 24 '17 19:10 RobAley

@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 avatar Nov 08 '17 06:11 AnrichVS

@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.

arist0tl3 avatar Nov 08 '17 23:11 arist0tl3

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.

thomasgodart avatar Jan 30 '18 13:01 thomasgodart

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:

mansours avatar Feb 07 '18 22:02 mansours

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 avatar Feb 22 '18 15:02 RobAley

@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;
}

rlansky avatar Apr 13 '18 14:04 rlansky

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.

jasonrundell avatar Apr 30 '18 14:04 jasonrundell

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.

martijndekuijper avatar Aug 06 '18 06:08 martijndekuijper

I think the 'bug' label should be added to this issue.

This is a matter of 'semantically correctness', but also of WYSIWYG.

stephanvierkant avatar Sep 21 '18 09:09 stephanvierkant

Ping @jhchen. Please mark this issue as a bug.

stephanvierkant avatar Oct 30 '18 07:10 stephanvierkant

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?

mohit18513 avatar Nov 05 '18 11:11 mohit18513

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 avatar Jan 26 '19 05:01 dantman

@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.

m00nk avatar Jan 26 '19 20:01 m00nk

Nothing happening on this issue? This is a bug imo

maggask avatar Mar 06 '19 10:03 maggask

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.

RobAley avatar Mar 06 '19 10:03 RobAley

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;
            }

maggask avatar Mar 08 '19 14:03 maggask

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.

tuancaraballo avatar Jun 06 '19 21:06 tuancaraballo