Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

I/6569: Remove array sorting flaws from table code #305

Merged
merged 6 commits into from
Apr 17, 2020
Merged

I/6569: Remove array sorting flaws from table code #305

merged 6 commits into from
Apr 17, 2020

Conversation

jodator
Copy link
Contributor

@jodator jodator commented Apr 15, 2020

Suggested merge commit message (convention)

Other: Introduce getColumnIndexes() helper method and properly handle index sorting. Closes ckeditor/ckeditor5#6569. Closes ckeditor/ckeditor5#6544.


Additional information

I don't have a good idea for a merge message as this:

  1. Introduces getColumnIndexes() helper method.
  2. Fixes getRownIndexes() bug
  3. Fixes other bugs when sorting indexes.

The PR is too big but reviews are long :(

@coveralls
Copy link

coveralls commented Apr 15, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 55ba7c1 on i/6569 into 7213f93 on master.

src/utils.js Outdated Show resolved Hide resolved
tests/commands/setheaderrowcommand.js Show resolved Hide resolved
@Reinmar Reinmar merged commit 99242fb into master Apr 17, 2020
@Reinmar Reinmar deleted the i/6569 branch April 17, 2020 15:29
return getFirstLastIndexesObject( indexes );
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

A couple of documentation issues here. Wording, wrong param name, lack of param name in the @param line, etc. I'm going to fix that on master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing 100+ table rows yields improper result Removing more than 7 rows in a 7 column table crashes editor
4 participants