Create numbered and bulleted lists #674

satsuper · 2014-06-23T02:12:06Z

This PR aims to add the creation of lists in a document. This only aims to support numbered and bulleted lists and not image bullet lists for now.

TODO

Editor UI buttons
Detect list style at selection
Ops for list insertion/removal
OT support
Convert selected text to a list or selected list back to text
Add feature flags and session constraints
Default styles for numbered and bulleted lists

peitschie · 2014-06-24T00:54:57Z

programs/editor/Tools.js

@@ -167,6 +169,9 @@ define("webodf/editor/Tools", [
                // Paragraph direct alignment buttons
                paragraphAlignment = createTool(ParagraphAlignment, args.directParagraphStylingEnabled);

+                // Paragraph direct alignment buttons
+                toggleLists = createTool(ToggleLists, true);


This will need it's own feature flag

peitschie · 2014-08-15T03:41:15Z

programs/editor/wodocollabtexteditor.js

@@ -177,6 +177,7 @@ var Wodo = Wodo || (function () {
            paragraphStyleEditingEnabled = isEnabled(editorOptions.paragraphStyleEditingEnabled),
            imageEditingEnabled = isEnabled(editorOptions.imageEditingEnabled, true),
            hyperlinkEditingEnabled = isEnabled(editorOptions.hyperlinkEditingEnabled, true),
+            listEditingEnabled = isEnabled(editorOptions.listEditingEnabled),


needs a 2nd parameter set to true to indicate the feature is still collab-unstable

kossebau · 2014-09-15T13:26:24Z

webodf/lib/gui/DefaultStyles.js

+ */
+
+/**
+ * This is the default style for numbered lists created by WebODF.


Don't we also need built-in defaults for those settings? What is currently happening for lists which styles which do not have all those settings set? What are the built-in defaults used there, and should those not be the same used here?

From memory a list with no style has no default fallback in webodf. The standard requires an implementation defined default http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#__RefHeading__1415150_253892949

I agree the same defaults should be used, maybe we could do what LO does and assign the bulleted style to any list without a style assigned.

Automatically assigning (as in: setting) might be too much for existing lists (that might break the intention of the original creator of the list). Just using it when rendering would be good, yes (but perhaps you meant this).

But I was also thinking about properties which are not set, so some built-in default values would be used. That needs to be defined somewhere as well. At least by how I understand the schema currently that can happen. Hm. Need to think more about that.

Yes I did mean to use them only for rendering.

kossebau · 2014-09-18T09:40:02Z

webodf/lib/odf/OdfUtils.js

+
+    this.isListElement = isListElement;
+
+    /**
     * Determine if the node is a text:list-item element.


please add or a <text:list-header>

Updated the comment

These operations work on the level of top level lists only Added tests for the new operations and changed the op test code to parse any op parameter containing "length" or "position" in the name as an integer

This handles transforms between Add/Remove list and other operations that modify the number of steps in the document. There are some unresolved conflicts with merging paragraphs and adding lists that return null as the transform result which required merge and split operations on lists to be resolved correctly. Added tests for the new parts of the op transform matrix and changed the op transform test code to parse any op parameter containing "length" or "position" in the name as an integer.

kogmbh-ci · 2014-09-19T04:31:01Z

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2258/

kossebau · 2014-09-22T12:07:27Z

webodf/tests/odf/OdfUtilsTests.js

@@ -393,6 +393,34 @@ odf.OdfUtilsTests = function OdfUtilsTests(runner) {
        testFontFamilyNameNormalizing("'serif'", "'serif'");
        testFontFamilyNameNormalizing("\"serif\"", "\"serif\"");
    }
+
+    function isListItemElement_ListItemOrListHeaderElements() {


Given the naming pattern $methodname_$testcase this should be isListItemOrListHeaderElement_ListItemAndListHeaderElements

kossebau · 2014-09-23T12:25:03Z

As you can see I have been hesitating so far to look at the code of the actual operations. Something in my mind told me I am not happy about them, but I could not say exactly what it was.

But now I know what made me unhappy: the lack of reasoning for the proposed operations and their spec. IMHO we should sit down this time and try to think about what operations there should be around lists, instead of going the painful try&error approach done in the initial prototyping of the WebODF editing (cough...removetext...cough).

So before continueing this PR I really would like to see a broader approach to the problem, before we spent our time on the actual implementation. If you agree please go here and add to the collection of ideas/plans around list operations:
https://github.com/kogmbh/WebODF/wiki/List-operations

satsuper · 2014-09-23T12:33:46Z

Have gone through quite a bit of design around this with @peitschie and thought about what UI actions we want to support so it's definitely not a trial and error approach. I will fill out this wiki page with the design for this PR and the future work that I mentioned on the mailing list.

kossebau · 2014-09-23T12:43:01Z

Forgot about the email, pardon (too much info at too many places 👀 ). Here for reference: https://open.nlnet.nl/pipermail/webodf/2014-August/000149.html (was there another one as well?)

Thanks for adding the notes also to the wiki page.

With the "trial and error" approach I was meaning that this time I would like to have a wider view of what the total number of operations around lists should be like. So far the proposed operations only cover a few use-cases, and they might fail with their specs and scopes once more use-cases are approached.

peitschie · 2014-09-23T13:20:48Z

Hrm...I'm completely on board with improving process, but I am a little disappointed that this desire couldn't have been expressed several months ago when we made it clear that this was a feature we were planning to deliver. Fulfilling this change in expectations will likely cost us a day or two of dev time in order to document the design decisions in the now-desired format.

I have two major concerns here:

This patch has been waiting 3 weeks for it's first round of commenting, and it's taken a further week to arrive at any solid conclusion. At that rate, it is likely to take weeks or even months to get any type of design approved.
As this is the first operation (or even feature) to ever undergo this type of documentation and review, I have no way to estimate timelines or workload that will likely result. As you can probably appreciate, this makes it difficult to secure project time to advance this case.

If we are going to undertake this, I really need some idea from you @kossebau about how high a priority (and therefore, how timely) the design & code reviews for this feature would be...

peitschie · 2014-09-23T13:29:14Z

If you agree please go here and add to the collection of ideas/plans around list operations:
https://github.com/kogmbh/WebODF/wiki/List-operations

A quick discussion with @satsuper, we think it will be best to put the design info into a standalone md file and raise a PR. The wiki interface doesn't allow any type of review interaction as will be necessary for this. On the PR, people can comment on specific lines and points, and see older revisions with comments.

Once the PR is signed off and everyone is happy, we can then transfer the markdown back to the wiki for permanent storage. Does that sound like a decent plan @kossebau @adityab ?

kossebau · 2014-09-23T13:34:54Z

we think it will be best to put the design info into a standalone md file and raise a PR.

Sounds like that should work best, yes. Please do.

adityab · 2014-09-23T13:37:07Z

Once the PR is signed off and everyone is happy, we can then transfer the markdown back to the wiki for permanent storage. Does that sound like a decent plan @kossebau @adityab ?

Sounds fine.

kossebau · 2014-09-23T13:42:02Z

I am a little disappointed that this desire couldn't have been expressed several months ago when we made it clear that this was a feature we were planning to deliver.

I can understand your disappointment very much. But then we/I have also learned quite a lot in all those months, and at least I draw the conclusion that ops need much more planning ahead and doing them on a case-by-case base is much more trouble in the end.

If we are going to undertake this, I really need some idea from you @kossebau about how high a priority (and therefore, how timely) the design & code reviews for this feature would be...

I will forward this to the person which rules me regarding that.... @th?

adityab · 2014-09-23T13:42:22Z

That said, If we are to move forward with this PR to at least land partial support for list editing (with a design discussion ongoing on the side of course), there's one thing that's very important - which is the ability to break out of a list and create a new paragraph after it.

kossebau · 2014-09-23T13:45:16Z

There could perhaps be something like an ops playground, where ops are living that are just around for experiments and have not yet seen any proper review.
And the same with the UI for that.

kossebau · 2014-09-23T13:46:17Z

Such a playground and the plugin infrastructure for that would also match what WebODF should ideally offer, being extensible by others with custom stuff. So would be a good testing for that.

peitschie · 2014-09-23T23:20:51Z

But then we/I have also learned quite a lot in all those months, and at least I draw the conclusion that ops need much more planning ahead and doing them on a case-by-case base is much more trouble in the end.

Yes. I agree completely on this. But, please don't overlook the fact that I have been involved in all aspects of this for the last 1.5 years however. After all, I have authored the majority of the documentation WebODF has on many of these subjects...

That said, If we are to move forward with this PR to at least land partial support for list editing (with a design discussion ongoing on the side of course), there's one thing that's very important - which is the ability to break out of a list and create a new paragraph after it.

By the time we add the ops + transforms required to do that, this PR would be far to large to review. We already have this planned in our roadmap, and we have operations designed to allow this. For this PR however, I would consider it unnecessary as it isn't a regression over current master.

There could perhaps be something like an ops playground, where ops are living that are just around for experiments and have not yet seen any proper review.

A plugin system would help alleviate the current symptoms if this was a stability or technical problem, or a question about the long term desirability of the feature. Currently however, this PR is facing a clear review time shortage, so a plugin system would provide no gain to anyone.

satsuper added the draft label Jun 23, 2014

satsuper changed the title ~~Create numbered and bulleted lists~~ [Draft] Create numbered and bulleted lists Jun 23, 2014

peitschie reviewed Jun 24, 2014
View reviewed changes

satsuper changed the title ~~[Draft] Create numbered and bulleted lists~~ Create numbered and bulleted lists Aug 14, 2014

peitschie reviewed Aug 15, 2014
View reviewed changes

kossebau reviewed Sep 15, 2014
View reviewed changes

satsuper mentioned this pull request Sep 16, 2014

Handle editing of partially selected lists #813

Open

kossebau reviewed Sep 18, 2014
View reviewed changes

peitschie force-pushed the master branch from ae2cfe1 to d761802 Compare September 18, 2014 15:30

Satvik Kumar added 4 commits September 19, 2014 11:17

Add UI buttons for list creation

8f8df7b

Add list controller and responding to cursor events

0520f0c

Add support for session constraints to list controller

2770489

Add unstable flags for the list editing feature

544529c

satsuper force-pushed the create-list branch from d18608f to ccb1f2b Compare September 19, 2014 04:18

Satvik Kumar added 3 commits September 19, 2014 14:26

Operations for adding and removing lists

06f0562

These operations work on the level of top level lists only Added tests for the new operations and changed the op test code to parse any op parameter containing "length" or "position" in the name as an integer

Default styles for numbered and bulleted lists

33566eb

satsuper force-pushed the create-list branch from ccb1f2b to 33566eb Compare September 19, 2014 04:28

kossebau reviewed Sep 22, 2014
View reviewed changes

satsuper mentioned this pull request Sep 29, 2014

List Editing Design #834

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create numbered and bulleted lists #674

Create numbered and bulleted lists #674

satsuper commented Jun 23, 2014

peitschie Jun 24, 2014

satsuper Aug 14, 2014

peitschie Aug 15, 2014

satsuper Aug 21, 2014

kossebau Sep 15, 2014

satsuper Sep 15, 2014

kossebau Sep 17, 2014

satsuper Sep 18, 2014

kossebau Sep 18, 2014

satsuper Sep 19, 2014

kogmbh-ci commented Sep 19, 2014

kossebau Sep 22, 2014

kossebau commented Sep 23, 2014

satsuper commented Sep 23, 2014

kossebau commented Sep 23, 2014

peitschie commented Sep 23, 2014

peitschie commented Sep 23, 2014

kossebau commented Sep 23, 2014

adityab commented Sep 23, 2014

kossebau commented Sep 23, 2014

adityab commented Sep 23, 2014

kossebau commented Sep 23, 2014

kossebau commented Sep 23, 2014

peitschie commented Sep 23, 2014

Create numbered and bulleted lists #674

Are you sure you want to change the base?

Create numbered and bulleted lists #674

Conversation

satsuper commented Jun 23, 2014

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kogmbh-ci commented Sep 19, 2014

Choose a reason for hiding this comment

kossebau commented Sep 23, 2014

satsuper commented Sep 23, 2014

kossebau commented Sep 23, 2014

peitschie commented Sep 23, 2014

peitschie commented Sep 23, 2014

kossebau commented Sep 23, 2014

adityab commented Sep 23, 2014

kossebau commented Sep 23, 2014

adityab commented Sep 23, 2014

kossebau commented Sep 23, 2014

kossebau commented Sep 23, 2014

peitschie commented Sep 23, 2014