1
\$\begingroup\$

My job is to supervise a junior developer, but I am mainly a javascript developer and my CSS skills are a bit basic. However, I spotted some things in his CSS code that seem unorthodox to me. He committed some CSS code similar to the one I post here:

<div class="label-item">
    <div class="label-image-container">
        <img src="www.aphoto.com"/>
    </div>
    <div class="label-info">

        <span class="label-name">John</span>
        <span class="description">John is an elephant</span>
        <div>
            <span class="size">1000 kg</span>
            <span class="age">10 years old</span>
        </div>
    </div>
    <div class="more-button">
        <span> ... </span>
    </div>
</div>

And the sass:

.label-item {
  margin: .8rem;
  display: grid;
  grid-template-columns: 4.4rem 1fr 1.8rem;
  grid-gap: 0 1rem;
  background-color: grey;
  align-items: center;
  width: 300px;
  height: 80px;
  position: relative;

  .label-image-container {
    width: 50px;
    height: 50px;
    display: inline-block;

    img {
      max-width: 50px;
      max-height: 50px;
      float: left;
      position: relative;
      top: 50%;
      left: 50%;
      transform: translate(-50%, -50%);
      height: auto;
    }
  }

  .label-info {
    display: grid;
    position: relative;
    grid-gap: .3rem;

    .label-name {
      font-size: 1.2rem;
      line-height: 1.5rem;
      height: 1.4rem;
    }
    .description, .size, .age {
      font-size: 1rem;
      display: inline-block;
      float: left;
    }

    .age {
      margin-left: .8rem;
      float: right;
    }
}
  .more-button {
    height: 100%;
    background-color: black;
    color: white;
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
    grid-row-end: span 1;
  }
}

this is a codepen of this code: https://codepen.io/anon/pen/QXadMP

My questions are:

a. Is grid positioning appropriate to alight and place elements inside a div that is not the main website container. I know that it works, but is this best practice?

b. Is nested grids considered to be best practice (.label-info inside .label item)?

c. Is using 'display: flex' appropriate to use just to align the content of a div in center?

\$\endgroup\$
2
\$\begingroup\$

I think the CSS looks fine. To your questions

a.) It doesn't matter which container has the grid property. It's CSS. It's not semantic.

b.) I am not sure if it is considered a best practice, but a lot of people seem to use it this way. Just google CSS nested grid and you will get quite some articles and videos on that topic. For example this and this.

c.) Counter question: Why not? CSS doesn't add any semantic meaning to HTML. It's just for styling. So, IMHO as long as it works and is scalable/maintainable it's fine.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

There are many things that, honestly, I don't consider best practices. In detail:

a) Using a grid seems not necessary there: such a structure could be realised with flex box only. It would have better compatibility.

b) I can't tell if nesting grids is considered a best practice either but this code does not get advantage of SASS in its nesting. Classes should have been written like this:

.label {
  &-item { }
  &-imageContainer { }
  &-info { }
  &-description { }
}

Moreover the div.label-info contains 2 spans and 1 div but, since all three are block-level elements, they should be divs.

c) Aligning with Flexbox is the right way but, as I said before, using Flexbox alone, without Grid, should have been a better practice.

I would also add that, since he's using Grid, all the floats are useless.

| improve this answer | |
\$\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.