reactabular / treetabular

Tree utilities (MIT)
MIT License
11 stars 6 forks source link

There no way to show multiple children which have sub-children #3

Closed softage0 closed 7 years ago

softage0 commented 7 years ago

I want to show the following hierarchy data:

I know you already mentioned on the README.md:

If there’s a parent relation, the children must follow their parent right after it.

But there's no way to place the children right after their parents on the above hierarchy, and treetabular shows wrong hierarchy like the below image: screen shot 2017-02-06 at 3 02 11 pm

Do you have plan to improve it?

The following code is my sample data:

      [
        {
          "id": "MNU00001",
          "parent": null,
          "name": "Configuration",
        },
        {
          "id": "MNU00101",
          "parent": "MNU00001",
          "name": "UserMangement",
        },
        {
          "id": "MNU00102",
          "parent": "MNU00001",
          "name": "MenuMangement",
        },
        {
          "id": "MNU00201",
          "parent": "MNU00102",
          "name": "TEST",
        },
        {
          "id": "MNU00103",
          "parent": "MNU00001",
          "name": "RoleMangement",
        },
        {
          "id": "MNU00202",
          "parent": "MNU00103",
          "name": "TEST",
        },
      ]
bebraw commented 7 years ago

Supporting that case would be good. I could accept a PR, but I won't likely resolve it myself (no funding for the project anymore).

softage0 commented 7 years ago

To implement the multiple sub parents, idField must be declared in getParents method.

And I'm considering the different approach to modify getParents method, as let it looks for future parent(searchingParent on the below code) not previousParent.

The code will looks like the below:

function getParents({
  index,
  idField = 'id',
  parentField = 'parent'
} = {}) {
  return (rows) => {
    const parents = [];
    let currentIndex = index;
    let cell = rows[index];
    let searchingParent;

    if (
      !cell ||
      typeof cell[parentField] === 'undefined' ||
      cell[parentField] === null
    ) {
      return parents;
    }

    while (cell) {

      if (typeof searchingParent === 'undefined') {
        searchingParent = cell[parentField];
      } else {
        if (searchingParent === cell[idField]) {
          parents.unshift(cell);
          searchingParent = cell[parentField];
        }
      }

      if (cell[parentField] === null) {
        break;
      }

      currentIndex -= 1;
      cell = rows[currentIndex];
    }

    return parents;
  };
}

export default getParents;

It makes the code simpler but there are 2 issues needs to be considered:

  1. idField must be declared. It's mandatory to implement the multiple sub parents. It can be set default as id same as other methods.
  2. If there's no matching parent of selected index, it will return empty array. In my opinion, it's correct return.

Please share your opinion. I'm not sure it's the right direction you intend to implement treetabular.

If you agreed I will implemented this way and send PR.

bebraw commented 7 years ago

idField must be declared.

That's fine. Most of the API requires it already so having it here doesn't hurt especially given the logic requires it.

If there's no matching parent of selected index, it will return empty array. In my opinion, it's correct return.

That can work. Another option would be to throw an error, but then the consumer would have to capture that. I'm fine with an empty array as long as it's documented.

@pzmudzinski Any thoughts on this?

pzmudzinski commented 7 years ago

Sounds good. Also I think _.isNil(cell[parentField]) is way more pretty than typeof cell[parentField] === 'undefined' || cell[parentField] === null ;)

softage0 commented 7 years ago

Cool :) Implementation is done. (@pzmudzinski thanks for your advice!) I will send PR after writing test cases and updating README.md.

softage0 commented 7 years ago

Pull has been requested.

This PR #4 includes the following features:

Please let me know if I missed or did wrong codes :)

bebraw commented 7 years ago

Nice work on a quick look. I'll try to provide a review in a day or two. If it's good enough, I'll likely just merge and cut a release. Thanks for all the work. 👍