Unable to indent a list

[Damian]

The indent feature does not allow indenting whole lists.

[Damian]

This is probably a new feature request?

We're getting more users ask about this feature. The current implementation of lists does not provide a way to indent the whole list an arbitrary number of times.

Is there a way we could support something like this? Perhaps we could wrap the list in a <div> or maybe a <blockquote>?

Any thoughts?

[Frederico Caldeira Knabben]

Considering that the indentation button is disabled if the selection is on the first item of the list, we could instead provide this feature for that case. So, it'll be enough to be in the first item to indent the entire list. If in other items, the current behavior is preserved.

[Garry Yao]

Replying to fredck:

So, it'll be enough to be in the first item to indent the entire list. If in other items, the current behavior is preserved.

That's exactly what featured by MS-Word in such case.

[Frederico Caldeira Knabben]

• Setting "criteria" to { tagNames : 1 } in the contains function is wrong, as will not work. Please change the function and, to have a cleaner API, use the "arguments" object, instead of expect having an Array or String being passed (the apply function will have to be used to call it).

• Load the following HTML and try to outdent the "B" item. It'll not work.

<ul>
	<li>A
		<ul>
			<li>B</li>
		</ul>
	</li>
	<li>C</li>
</ul>

[Garry Yao]

Replying to fredck:

Please change the function and, to have a cleaner API, use the "arguments" object, instead of expect having an Array or String being passed (the apply function will have to be used to call it).

I don't quite understand the idea behind this.

• Load the following HTML and try to outdent the "B" item. It'll not work...

It should be considered as an edge-case, in fact, when it's impossible to 'outdent' the whole list anymore (in above case), we should also let it outdent the item instead.

[Frederico Caldeira Knabben]

Replying to garry.yao:

I don't quite understand the idea behind this.

That's not important for now. Your last patch simplified it in a way that the feature I was mentioning can be implemented in the future backwards compatible.

It should be considered as an edge-case

No, that's not. This is a very common case when working on re-defining the structure of a complex list.

[Frederico Caldeira Knabben]

The following TC is not working anymore:

1. Load the following HTML and make the marked selection:

<ul>
	<li>Item 1</li>
	<li>Item [2</li>
	<li>Item] 3</li>
	<li>Item 4</li>
</ul>

2. Hit the "Increase Indent" button.

After patch results: The entire list gets indented.

Current/Expected results: "Item 2" and "Item 3" forms a second level list under "Item 1".

Note that the proposed list indentation feature must work when the selection is totally inside the first element in the list, only.

[Frederico Caldeira Knabben]

1. Load the following HTML and make the marked selection:

<ul>
	<li>[A</li>
	<li>B]</li>
	<li>C</li>
</ul>

2. Hit the "Increase Indent" button.

Expected results: The entire list gets indented.

After patch results: The "C" item disappears and no indentation happens.

[Frederico Caldeira Knabben]

Another TC:

1. Load the following HTML:

<p>Para 1</p>
<ul>
	<li>A</li>
	<li>B</li>
	<li>C</li>
</ul>
<p>Para 2</p>

2. Hit CTRL+A to select all.
3. Click the "Increase Indent" button.

Expected results: the paragraphs and the list get indented.

After patch results: the paragraphs get indented, but the list stays in-place, having its items contents indented.

[Garry Yao]

...when the selection is totally inside the first element in the list, only.

...when the selection start from the first element in the list.

Another TC: ... After patch results: the paragraphs get indented, but the list stays in-place, having its items contents indented.

This's is yet another feature and requires a drastic change (domIterator), which we should void at the current moment.

[Frederico Caldeira Knabben]

1. Load the following HTML:

<ul style="margin-left: 40px;">
	<li>A</li>
	<li>B</li>
	<li>C</li>
</ul>

2. Put the caret in the "B" item.
3. Click the "Increase Indent" button.

Expected results: the "B" item becomes e sub-list of "A".

After patch results: the "B" item becomes a sub-list of "A", but it also takes it's margin-left style, which should happen only in a second click.

[Garry Yao]

The above bug is already existed in trunk, I've opened #5372 for it.

[Frederico Caldeira Knabben]

1. In FF, load the following HTML:

<ul>
	<li>
		A</li>
	<li>
		B</li>
</ul>

2. Hit CTRL+A to select all.
3. Click the "Increase Indent" button.

A js error is thrown and bookmark nodes are outputted in the source view.

[Frederico Caldeira Knabben]

This patch breaks the indentation features... the "Decrease Indentation" never gets enabled, even on normal paragraphs.

[Frederico Caldeira Knabben]

Unfortunately the TCs of comments 12 and 14 are still broken.

[Frederico Caldeira Knabben]

Replying to fredck:

Unfortunately the TCs of comments 12 and 14 are still broken.

Ops, my fault... comment 12 has already been addressed with a new ticket, as explained by Garry. But I can still reproduce the comment:14 issue.

[Frederico Caldeira Knabben]

It's nice to see the final patch here!

My only concern is regarding the changes in contents.css. We should not make changes in that file, because that's a "user file". Users should point to custom files, which may not have it. Other than that, the patch has the 40px value hardcoded, but it depends on the settings instead. So, those CSS rules should go inside the plugin file, sensitive to the editor settings.

[Garry Yao]

Replying to fredck:

My only concern is regarding the changes in contents.css. We should not make changes in that file

Fortunately the list problem affects only wysiwyg mode instead of the output page, so that's a good idea to keep those reset styles inside our internal style sheet.

Other than that, the patch has the 40px value hardcoded, but it depends on the settings instead.

Not true, it's only a reset for raw HTML list, doesn't impact our indent feature.

[Frederico Caldeira Knabben]

When committing, please remove the listStyleText variable as it's not needed, having the string passed to the addCss function directly. Btw, there is extra space in "editor .addCss", which can also be corrected.

Please commit this on in the 3.3.x branch.

[Garry Yao]

Fixed with [5307] on 3.3.x branch.