Display Object as a node tree in HTML with clickable trace back to root node

Question

I was given the following code and instructions for a technical test for a potential technical role I had applied for:

var input = {
    label: 'Earth',
    type: 'planet',
    children: [{
        label: 'Australia',
        type: 'country',
        children: [{
            label: 'Sydney',
            type: 'city',
            children: [{
                label: 'Nuix HQ',
                type: 'office',
                children: []
            }]
        }]
    }, {
        label: 'USA',
        type: 'country',
        children: [{
            label: 'San Franscisco',
            type: 'city',
            children: [{
                label: 'Nuix SF',
                type: 'office',
                children: []
            }]
        }, {
            label: 'Philadelphia',
            type: 'city',
            children: [{
                label: 'Nuix Philly',
                type: 'office',
                children: []
            }]
        }]
    }]
};

Task:

Using Javascript please render HTML element that displays above JSON data. It should reflect parent-child relationship by appropriate element nesting and indentation. The structure should be right-justified, meaning the indentation should start from the right and move to the left as the nodes descend. Additionally, when a node is clicked, it should print (to console) all labels on a path from clicked node to the root (Earth).

CSS visual styling is optional (nice to have). You can use any open-source utilities and libraries available in the fiddle, however we also value simplicity of the solution. The solution should be delivered in a fork of this fiddle.

I supplied the additional code as a fork of the original code:

Javascript:

let node = 1;
var result = [];
var arrNodeParent = [];
var arrNodeName = [];
tree = (lvl, parent = 0, node, data) =>
{
    let div = `<div class="node lvl${lvl}" node="${node}" parent="${parent==0?0:parent-1}">${data.label}</div>`;

    arrNodeParent[node] = parent==0?0:parent-1;
    arrNodeName[node] = data.label;

  n = node+=1;
  (data.children.length)&&data.children.forEach(e=>tree(lvl+1, node, n, e));
  result[node]=div;
}

tree(0,0,node,input)

$('.tree').append(...result);

$(".node").click(function(){
    complete = false;
        currNode = $(this);
  while(!complete){
    n = +currNode.attr("node");
    p = +currNode.attr("parent");
        console.log(arrNodeName[n]);
    currNode = $("div[node='"+arrNodeParent[n]+"']");
    complete = (p==0);
  }  
});

HTML:

CSS:

.tree {
width: 100%;
}
.node {
  font-size: 12px;
  padding: 3px;
  margin-bottom: 1px;
  border: 1px solid black;
  background-color: rgb(255, 255, 128);
  width: 100px; 
  margin-left: auto;
  margin-right: 0px;
}
.node:hover {
  cursor: pointer;
}
.lvl0 {
  margin-right: 0;
}
.lvl1 {
  margin-right: 20px;
}
.lvl2 {
  margin-right: 40px;
}
.lvl3 {
  margin-right: 60px;
}
.lvl4 {
  margin-right: 80px;
}
.lvl5 {
  margin-right: 100px;
}

Now, I was happy with my result, I felt I passed the brief... The client has come back saying I didn't meet the brief... I'm confused here... What specifically have I missed? And is there anything I could have done? I have asked for feedback, but I have yet to receive anything in response... Could someone possibly shed some light?

Answer 1

It's always hard to guess, what the interviewer actually wanted. However, here are some thoughts mostly on style and markup.

---

Arrow function

You're using an arrow function in global scope, where it doesn't add value by binding this:

tree = (lvl, parent = 0, node, data) => {}

I would stick with a normal function here. It is also more readable. There's a discussion on SO – When should I use Arrow functions in ECMAScript 6? – which also explain why and where arrow functions are beneficial.

Readability

Here are some parts, that are hard to read:

arrNodeParent[node] = parent==0?0:parent-1;
n = node+=1;
(data.children.length)&&data.children.forEach(e=>tree(lvl+1, node, n, e));

Especially the last line. While it makes use of JavaScript's way to handle logical expressions, I would say: This is not Code Golf but production code. So readability and quick understanding of the code, even tomorrow, is more important then saving a few bytes by creating one-liners.

Inconsistent use of keywords

Sometimes you declare variables using let, often you don't. Sometimes const could be used instead. Also, think twice before using var and check whether globals are really necessary for your function.

Indentation

Your Indentation is inconsistent. Sometimes it's two, sometimes it's four characters. Some lines are totally off, like here:

    complete = false;
        currNode = $(this);
  while(!complete){

HTML

Semantics/nesting

It should reflect parent-child relationship by appropriate element nesting and indentation.

A div and also span are the elements with no semantic meaning, which make them not the first choice for a relationship like that. You also don't nest the elements to reflect that, you simply list them:

<div class="tree">
    <div class="node lvl0" node="1" parent="0">Earth</div>
    <div class="node lvl1" node="2" parent="1">Australia</div>
    <div class="node lvl2" node="3" parent="2">Sydney</div>
</div>

Adding a class like level-n does not add semantics to HTML.

You can change the markup to a nested list for example:

<ul>
    <li>
        Earth
        <ul>
            <li>
                Australia
                <ul>
                    <li>Sydney</li>
                </ul>
            </li>
        </ul>
    </li>
</ul>

Invalid markup

You add two attributes node and parent to each element. These aren't valid HTML attributes. If you want to use your own custom ones, use data-*-attributes, like:

<li data-node="1" data-parent="0">Earth</li>

jQuery

Yes, the task says, you can use any open source library you need. But it also says they like simplicity. So adding 87 kB of JavaScript to simply attach an event handler and to select/create a few HTML elements seems like a huge overhead to me. If the library doesn't help building the actual tree, I would stick to vanilla JavaScript in this case.