Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Allow for the indention and outdention of multiple selected headings at once in the structured headings plugin #422

Merged
merged 16 commits into from Aug 7, 2013

Conversation

NickMcL
Copy link
Contributor

@NickMcL NickMcL commented Aug 5, 2013

In the structured headings plugin currently, if a user selects multiple headings at once and uses the indent or outdent tool, the tool will only be applied to the last heading in the selection. Fix this so that the tool applies to every heading in the selection if allowable.

@NickMcL
Copy link
Contributor Author

NickMcL commented Aug 5, 2013

Indenting or outdenting multiple headings in a selection at once now works as expected. When applying an indent or outdent to multiple headings at once, the plugin will loop through each heading in the selection and apply the specified action to the heading if it does not create an invalid heading structure.

As part of adding this functionality to the structured headings plugin, I also had to fix how indent and outdent tools work when applied to normal lists. I made it so that the indent and outdent actions will only be applied to a list if every node in the selection is within the same list. This is tested by the _selectionOnlyInList function. In addition, I had to modify the behavior of getCommonParentList so that it can be used to find either the closest or furthest common parent of the passed list nodes.

Also, as part of this ticket, I added a large amount of tests for both indenting and outdenting with multiple headings to try to cover all of the various cases where those actions could take place.

if (parentList.length === 0 || parentList[0] !== rootList) {
return null;
haveCommonParent = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this ticket, the getCommonParentList function was trying to return null from within an internal function, so it wasn't actually ever able to return null and would always return a parent list whether all of the nodes were in it or not. I'm surprised this wasn't causing other issues elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. It looks like this would have broken lots of things if a user had an odd selection when trying to indent a list.

@NickMcL
Copy link
Contributor Author

NickMcL commented Aug 6, 2013

I went back and added some more tests specifically for ensuring that the getCommonParentList function that I modified works as expected when indenting/outdenting a list or converting list types.

headingManager = this,
shouldRaise = (upOrDown === 'up'),
i,
i_start = (shouldRaise ? selection.rangeCount - 1 : 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be camelCase'd

@winhamwr
Copy link
Member

winhamwr commented Aug 7, 2013

I love the tests on this. There are definitely lots of cases to consider and you did a great job covering them. After the small camelCase change, this will be ready to merge in.

@NickMcL
Copy link
Contributor Author

NickMcL commented Aug 7, 2013

I fixed the camel case issue you pointed out.

winhamwr added a commit that referenced this pull request Aug 7, 2013
Fixes #422 Allow for the indention and outdention of multiple selected headings at once in the structured headings plugin
@winhamwr winhamwr merged commit f17f1d1 into wymeditor:master Aug 7, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants