Ticket #1231 (closed New Feature: fixed)

Opened 7 years ago

Last modified 7 years ago

Use CSS styles for indent and outdent

Reported by: martinkou Owned by: martinkou
Priority: Normal Milestone: FCKeditor 2.5 Beta
Component: Core : Styles Version: SVN (FCKeditor) - Retired
Keywords: Cc:

Description

The current indent/outdent implementation uses built-in browser commands for indentation, which in turn uses <blockquote> or <ul>/<ol> tags to indent or outdent text. Although the behavior is consistent between browsers, this approach produces unnecessary HTML tags.

A better way to implement indenting is by using the margin-left and margin-right CSS properties. They do not introduce extra HTML tags and are not affected by any styles that might be declared with <blockquote> in mind (see #1197).

The new indent/outdent implementation should take into account of right-to-left languages as well.

Attachments

arrayModel.html (6.0 KB) - added by martinkou 7 years ago.
Proof-of-concept implementation of the array list model, and doing indentations with it.

Change History

comment:1 Changed 7 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:2 Changed 7 years ago by fredck

To make it extremely powerful, it would be nice to be able to use CSS classes to indent too.

By default, we would use "margin-left:40px" for indentation, increasing its value by 40px for each new indentation.

But, to achieve XHTML 1.1 needs, we should avoid using the "style" attribute and work with "class" only. And here is the tricky part of it.

My proposal is having a dedicated configuration option for it, called "IndentClasses". By default it would be empty:

FCKConfig.IndentClasses = '' ;

In this case, the "style" attribute is used.

Then, in my XHTML 1.1 web site, I define the following classes in the CSS:

.Indent1
{
  margin-left: 40px;
}
.Indent2
{
  margin-left: 80px;
}
.Indent3
{
  margin-left: 120px;
}

In the editor, I just change the configuration to:

FCKConfig.IndentClasses = 'Indent1,Indent2,Indent3' ;

Voila, the editor automatically starts using the "class" attribute instead of "style". It would make it possible to apply three levels of indentation. Reached the last one, the indentation button should become disabled.

The idea is, providing a XHTML 1.1 compliant editor, without loosing the powerful and full featured interface.

I hope all this thing makes sense.

comment:3 Changed 7 years ago by martinkou

I've got it mostly working in r835, but there's still a list indenting case where it isn't working:

<ul>
 <li><div>item1</div></li>
 <li><div>item2</div></li>
</ul>

Selecting the items and trying to indent/outdent still does not work. What's funny is that I've actually written some code to account for this case, but the iterator does not seem to iterate over the items.

I'll continue to investigate to see what can be done, and whether it's a bug with the iterator.

comment:4 follow-up: ↓ 7 Changed 7 years ago by fredck

In the above <ul> example, the iterator is retrieving the <div>s, as expected, as those are the block elements for the selection inside the list.

One idea would be checking for the block element returned by the iterator, if it is the only element inside its parent <li>. In this case, the element to be indented is the <li>.

BTW, we must never produce things like this:

<ul>
    <ul>
        <li>item1</li>
    </ul>
    <li>item2</li>
</ul>

UL elements must have only LI children (per DTD definition). So, it should not be possible to indent the first item in a list, as it it doesn't have an LI before of it to hold the indentation.

I have the impression that we'll have people complaining about it.

comment:5 Changed 7 years ago by fredck

We are having style="margin-left: 0px;" being left in the source when outdenting. If the indentation value is 0, the style property must be removed, and also the style attribute if it remains empty.

comment:6 Changed 7 years ago by fredck

Also, in the rare situations where the margin size has an value which is not a multiple of the offset value (it may happen when changing the editor settings), we should make the algorithm in a way that things get aligned to the new "grid".

For example, today, if the margin is set to 45 and the editor is configured to 40, when outdenting, it will become 5. Outdenting again will go to 0. Ideally, it should move to to 40, and then to 0, as the offset configured is actually defining a virtual "grid" for the indentation.

If I'm not wrong, we should have something like this (if the division doesn't return an integer, which I'm not sure):

newValue = Math.ceil( currentValue / offset ) - offset

comment:7 in reply to: ↑ 4 Changed 7 years ago by martinkou

I see your point, XHTML 1.1 does not allow a <ul> to be placed directly under another <ul> (and same for <ol>).

So if we were to be XHTML 1.1 compliant, the right way to do list indentation would be:

<ul>
  <li>item1
    <ul>
      <li>indented item2</li>
      <li>indented item3</li>
    </ul>
  </li>
</ul>

But compared to simply indenting by placing a <ul> under another <ul>, this approach creates a number of non-trivial cases for the outdenting operation, if we were to model outdenting as simple DOM tree manipulations. For example, if I were to select "indented item2" and outdent it, the resulting DOM tree would be:

<ul>
  <li>item1</li>
  <li>indented item2
    <ul>
      <li>indented item3</li>
    </ul>
  </li>
</ul>

And that's just one of the cases where you can't just do a "move all children to another node" kind of simple DOM tree operation to accomplish outdenting. I'm sure there are many more cases to handle.

What I want to say here, is that for operating on a list, a DOM tree is not a good model. Visually, a list in a document resembles more like a linear structure with indenting rather than a tree with intermediate node telling what kind of list item it contains. So, if we can convert a list node's DOM tree into a linear structure, list operations (which seems to have more direct relation to the visual appearance of a list, rather than its tree structure) should be easier.

Consider this list tree in DOM:

<ul>
  <li>item1
    <ul>
      <li>indented item2</li>
      <li>indented item3</li>
      <li>indented item4</li>
    </ul>
  </li>
  <li>item5</li>
</ul>

As I said before, outdenting an arbitrary selection within this list can be unwieldy if you perform your operations directly on the DOM tree. So, let's convert that to an array in pseudo code:

[
  {parent:<ul>, indent:0, contents:item1},
  {parent:<ul>, indent:1, contents:indented item2},
  {parent:<ul>, indent:1, contents:indented item3},
  {parent:<ul>, indent:1, contents:indented item4},
  {parent:<ul>, indent:0, contents:item5}
]

Now, say I want to outdent item3, I can just perform a simple operation in the list...

[
  {parent:<ul>, indent:0, contents:item1},
  {parent:<ul>, indent:1, contents:indented item2},
  {parent:<ul>, indent:0, contents:indented item3},
  {parent:<ul>, indent:1, contents:indented item4},
  {parent:<ul>, indent:0, contents:item5}
]

Of course I'd still have to convert the list back to a DOM node for it to be displayed. But this conversion would have much less special cases to handle. So, if I were to convert the outdented list's array representation back to a DOM tree, I would have:

<ul>
  <li>item1
    <ul>
      <li>indented item2</li>
    </ul>
  </li>
  <li>indented item3
    <ul>
      <li>indented item 4</li>
    </ul>
  </li>
  <li>item5</li>
</ul>

The conversion can be done naturally without all the split nodes or moving nodes in a weired way operations. The only constraint that needs to be placed on the array data structure is that the indent level of the next item can only be at most +1 of its previous item.

This array list model and array <-> DOM tree conversion logic should be of great help for solving #1178 as well.

Replying to fredck:

In the above <ul> example, the iterator is retrieving the <div>s, as expected, as those are the block elements for the selection inside the list.

One idea would be checking for the block element returned by the iterator, if it is the only element inside its parent <li>. In this case, the element to be indented is the <li>.

BTW, we must never produce things like this:

<ul>
    <ul>
        <li>item1</li>
    </ul>
    <li>item2</li>
</ul>

UL elements must have only LI children (per DTD definition). So, it should not be possible to indent the first item in a list, as it it doesn't have an LI before of it to hold the indentation.

I have the impression that we'll have people complaining about it.

Changed 7 years ago by martinkou

Proof-of-concept implementation of the array list model, and doing indentations with it.

comment:8 Changed 7 years ago by martinkou

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed with [846].

Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy