7
\$\begingroup\$

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:

enter image description here

enter image description here

enter image description here

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 < 5 && textboxContent.length) {
      textbox.value = '';

      items.push({
        textboxContent,
        color: ''
      });

      items.sort((a, b) => {
        if (a.textboxContent > b.textboxContent) {
          return 1;
        } else if (a.textboxContent < b.textboxContent) {
          return -1;
        } else {
          return 0;
        }
      });

      setColors();
    }
  }

  function setColors() {
    items.forEach((item, index) => {
      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 (pyramidPanel.firstChild) {
      pyramidPanel.removeChild(pyramidPanel.lastChild);
    }
  }

  addNewItem.addEventListener('click', () => {
    updateItems();
    emptyPanel();
    updateView();
  });

  removeLastItem.addEventListener('click', () => {
    items.pop();
    emptyPanel();
    updateView();
  });
})();
body {
  background-color: #ebebeb;
}

.pyramid-wrap {
  margin: 20px auto;
  max-width: 750px;
  background-color: whitesmoke;
  border-radius: 8px;
  padding: 5px 10px;
}

.nav-item {
  margin-bottom: 20px;
}

#content-new-item {
  display: inline-block;
  width: 300px;
}

.nav-item *:nth-child(n + 1) {
  margin-right: 15px;
  color: black;
}

.pyramid-item {
  height: 0;
  margin-bottom: 10px;
  position: relative;
}

.pyramid-content {
  position: absolute;
  top: 50%;
  left: 50%;
  transform: translate(-50%, 70%);
  font-weight: 800;
  color: white;
  letter-spacing: 2px;
}

.nav-item-control:focus {
  box-shadow: 0 0 3px #555;
  color: #555;
}

button.nav-item-control:hover {
  background-color: #787878;
  color: #eee;
  border: 0;
}

input:hover::-webkit-input-placeholder {
  color: #dedede;
}

input:hover::-moz-placeholder {
  color: #dedede;
}

input:hover:-ms-input-placeholder {
  color: #dedede;
}

button.nav-item-control:active {
  color: #fff;
  box-shadow: 0 0 6px white;
}
<div class="pyramid-wrap">
  <div id="pyramid-panel"></div>
  <nav>
    <div class="nav-item">
      <input class="nav-item-control" type="text" id="content-new-item" placeholder="Please enter the text content for next pyramid item ..." />
      <button class="nav-item-control" id="add-new-item">Add new pyramid item</button>
    </div>
    <div class="nav-item">
      <button class="nav-item-control" id="remove-last-item">Remove last pyramid item</button>
    </div>
  </nav>
</div>

What do you think about my idea of computing the margins and widths of each div?

An alternative would have been to write multiple CSS-classes and then adding the corresponding CSS-class.

Which approach is better and for what reason? Are there points which could be improved? Or done better some different way?

\$\endgroup\$
  • 1
    \$\begingroup\$ Is the pyramid definitely limited to 5 levels? Must the result be responsive? Is the layout and the HTML markup part of the task or free to chose? \$\endgroup\$ – insertusernamehere Mar 19 '17 at 13:13
  • \$\begingroup\$ Beside the task description (received via mail on Friday afternoon) I had three screenshots similar to the ones I published in my question. So I started on Sunday morning and based my HTML / layout upon that. Also I choose to limit my implementation to five levels because of the screenshots. It is demonstrated with five elements there. Responsiveness isn't mentioned in the requirements. So I decided to implement only the mentioned points and to supply as early as possible. Beside that: I will have an exam by the end of this week. So I liked to get it done for becoming my head free. \$\endgroup\$ – michael.zech Mar 20 '17 at 3:53
  • \$\begingroup\$ But anyway: Many thanks for your answer. Awesome stuff to think about. I they don't call me back again then I have definitely learned a few things. \$\endgroup\$ – michael.zech Mar 20 '17 at 3:59
  • \$\begingroup\$ If I would ask this task as part of an interview for a frontend developer, these are the things I would look for: "Can the applicant create a prototype from a vague customer request?" "Does the applicant see limitations and edge cases?" "Is the source readable and maintainable? How are DOM operations handled?" "Is the markup semantic?". However, be careful assuming that mockups show you all use cases or limitations of an application. I would probably add unclear things as questions, like: "Is this limited to 5 elements?" or "Are you sure you want to sort numbers alphabetically?". \$\endgroup\$ – insertusernamehere Mar 20 '17 at 10:40
  • \$\begingroup\$ Valuable thoughts to consider. Thanks a bunch for your advice. :) I really appreciate it. \$\endgroup\$ – michael.zech Mar 20 '17 at 11:48
7
\$\begingroup\$

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:

  1. aB
  2. 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.

  1. If you set all styles in CSS you can get rid of all the calculation.
  2. 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
});

  1. 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:

<button type="button" class="nav-item-control" id="add-new-item">Add new pyramid item</button>

fieldset

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

<fieldset class="nav-item">
    <input type="text">
    <button type="button">Add new pyramid item</button>
</fieldset>

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');
\$\endgroup\$
1
\$\begingroup\$

A more modern approach is to utilize a javascript framework.

I have written this similar feature in React which you can see here: https://codesandbox.io/s/qz98rxy8ow

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.