Create a dynamic growing pyramid

In connection with a job application I have to progress the following task:


Create a dynamic growing pyramid. The structure has to be sorted
alphabetically (concerning the inserted values).
It has to have the following controls:



-
Textbox for inserting texts (used by the blocks of the pyramid).

-
Button which adds the text to the list. Furthermore updates the pyramid.

-
Button which removes the last block of the pyramid. Furthermore updates the pyramid.



You are allowed to use HTML, CSS and JavaScript.

Here are screenshots for to demonstrate how it is meant:

Here the code I've written:



(function() {
var items = [];
var addNewItem = document.querySelector('#add-new-item');
var removeLastItem = document.querySelector('#remove-last-item');
var textbox = document.querySelector('#content-new-item');
var pyramidPanel = document.querySelector('#pyramid-panel');
var colors = ['orange', 'blue', 'red', 'green'];

function updateItems() {
var textboxContent = textbox.value

if (items.length {
if (a.textboxContent > b.textboxContent) {
return 1;
} else if (a.textboxContent {
item.color = colors[index % colors.length];
});
}

function updateView() {
items.forEach((item, index) => {
let div = document.createElement('div');
let span = document.createElement('span');
let textNode = document.createTextNode(item.textboxContent);

div.setAttribute('class', 'pyramid-item');
span.setAttribute('class', 'pyramid-content');

div.style.borderRight = '75px solid transparent';
div.style.borderBottom = 50px solid ${item.color}`;
div.style.borderLeft = '75px solid transparent';
div.style.marginLeft = ((4 - index) * 75) + 'px';
div.style.width = (150 * index) + 'px';

span.appendChild(textNode);
div.appendChild(span);

pyramidPanel.appendChild(div);
});
}

function emptyPanel() {
while (pyram

A few thoughts on the task and your solution:

Limited to 5 elements

You're limiting the pyramid to 5 elements. The task doesn't have a restriction like that. I see two possibilities here:

• Do not limit your solution.

• Improve the UX of your solution and explain, why you have a limit included.

When one reaches the maximum, your program seems broken. There's no message explaining, that I can't add more items and why. Also disabling the form elements would be helpful, when going with the restriction.

Empty input

I can't add "empty" items. But I can add items containing whitespaces only, like " ". Be more strict:

if (/\S/.test(textboxContent)) {}

or allow an empty input as well. However, a message explaining why I can't add an item would be nice.

Sorting

The task says, sort alphabetically. It doesn't say anything about case sensitivity but your solution sorts like this:

• aB

• aa

If this is not desired, use toLowerCase() or .localeCompare(), like:

return a.textboxContent.localeCompare(b.textboxContent);

Numbers are sorted lexicographic/alphabetically already. So 10 comes before 2 and fulfills the task.

Styles

I would recommend to decouple the style/CSS from the JavaScript as much as possible. Especially when you handle only 5 items. You don't even need to handle specific classes for each item in JavaScript. Use the nth-child() selector to style each tile:

.pyramid-item:nth-child(1) { color: orange; }
.pyramid-item:nth-child(2) { color: green; }

Which will simplify the next part:

Update View

updateView is doing a lot of things, over and over.

• If you set all styles in CSS you can get rid of all the calculation.

• Don't create the same DOM elements again and again. Create them once and store them along with your item:

let pyramidElement = document.createElement('div');
[…]

items.push({
    content: textboxContent,
    element: pyramidElement
});

• Don't call pyramidPanel.appendChild() on each iteration. Create a DocumentFragment, because:

it […] keeps the recalculation, painting and layout to a minimum.

Finally, your method could be reduced to this:

function updateView() {
    let fragment = document.createDocumentFragment();

    items.forEach((item, index) => {
        fragment.appendChild(item.element);
    }

    pyramidPanel.appendChild(fragment);
}

See als Should I use document.createDocumentFragment or document.createElement for more details.

RemoveLastItem

Why are you re-creating the whole thing, when you remove the last item? Simply remove that specific element and you're done, like:

let item = items.pop();
pyramidPanel.removeChild(item.element);

Selectors

All selectors use an id to address elements. You are using document.querySelector, which does the job. But using the explicit selector for this task document.getElementById will improve performance, as it is 60% (Chrome), 70% (Safari) or 99% (Firefox) faster. Test it yourself.

Markup

nav vs. form

From w3.org: "HTML/Elements/nav":


The element represents a section with navigation links.

Your nav-element clearly is not such a section. Also you're using form elements outside a form without linking them to one. So instead of nav use a form-element.

Keep in mind, that you have to suppress the form submission.

button

A button without a specified type is a submit button by default. Spend them the type-attribute:

Add new pyramid item

fieldset

As one input and one button act together, you could group them as a fieldset:

    
    Add new pyramid item

Variable Naming

Try to use descriptive variable names all the time, not only sometimes. pyramidPanel is good. But div isn't, instead use:

let pyramidElement = document.createElement('div');

StackExchange Code Review Q#158189, answer score: 7