Painéis Flex em CSS e JS

Oct 31 2020

Eu estava seguindo o desafio de 30 dias de Wes Bos JS, então HTML e CSS são principalmente copiar e colar, gostaria de receber feedback sobre JS (principalmente). Obrigado.

const panels = document.querySelectorAll('.panel');

panels.forEach(panel => panel.addEventListener('click', () => {
  const isOpen = panel.classList.contains('open');
  panels.forEach(panel => panel.classList.remove('open'));
  if(!isOpen) {
    panel.classList.add('open');
  }
}));

panels.forEach(panel => panel.addEventListener('transitionend', e => {
  if(e.propertyName.includes('flex')) {
    panels.forEach(panel => {
      if(panel.classList.contains('open')) {
        panel.classList.add('open-active');
      } else {
        panel.classList.remove('open-active');
      }
    });
  }
}));

html {
  box-sizing: border-box;
  background: #ffc600;
  font-family: 'helvetica neue';
  font-size: 20px;
  font-weight: 200;
}

body {
  margin: 0;
}

*,
*:before,
*:after {
  box-sizing: inherit;
}

.panels {
  min-height: 100vh;
  overflow: hidden;
  display: flex;
}

.panel {
  background: #6B0F9C;
  box-shadow: inset 0 0 0 5px rgba(255, 255, 255, 0.1);
  color: white;
  text-align: center;
  align-items: center;
  /* Safari transitionend event.propertyName === flex */
  /* Chrome + FF transitionend event.propertyName === flex-grow */
  transition:
    font-size 0.7s cubic-bezier(0.61, -0.19, 0.7, -0.11),
    flex 0.7s cubic-bezier(0.61, -0.19, 0.7, -0.11), background 0.2s;
  font-size: 20px;
  background-size: cover;
  background-position: center;

  flex: 1;
  display: flex;
  justify-content: center;
  flex-direction: column;
}

.panel1 {
  background-image: url(https://source.unsplash.com/gYl-UtwNg_I/1500x1500);
}

.panel2 {
  background-image: url(https://source.unsplash.com/rFKUFzjPYiQ/1500x1500);
}

.panel3 {
  background-image: url(https://images.unsplash.com/photo-1465188162913-8fb5709d6d57?ixlib=rb-0.3.5&q=80&fm=jpg&crop=faces&cs=tinysrgb&w=1500&h=1500&fit=crop&s=967e8a713a4e395260793fc8c802901d);
}

.panel4 {
  background-image: url(https://source.unsplash.com/ITjiVXcwVng/1500x1500);
}

.panel5 {
  background-image: url(https://source.unsplash.com/3MNzGlQM7qs/1500x1500);
}

/* Flex Children */
.panel>* {
  margin: 0;
  width: 100%;
  transition: transform 0.5s;
  flex: 1 0 auto;
  display: flex;
  align-items: center;
  justify-content: center;
}

.panel *:first-child {
  transform: translateY(-100%);
}

.panel *:last-child {
  transform: translateY(100%);
}

.panel.open-active *:first-child,
.panel.open-active *:last-child {
  transform: translateY(0);
}

.panel p {
  text-transform: uppercase;
  font-family: 'Amatic SC', cursive;
  text-shadow: 0 0 4px rgba(0, 0, 0, 0.72), 0 0 14px rgba(0, 0, 0, 0.45);
  font-size: 2em;
}

.panel p:nth-child(2) {
  font-size: 4em;
}

.panel.open {
  font-size: 40px;
  flex: 5;
}

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Flex Panels 💪</title>
  <link href="https://fonts.googleapis.com/css?family=Amatic+SC" rel="stylesheet" type="text/css">
</head>
<body>

  <div class="panels">
    <div class="panel panel1">
      <p>Hey</p>
      <p>Let's</p>
      <p>Dance</p>
    </div>
    <div class="panel panel2">
      <p>Give</p>
      <p>Take</p>
      <p>Receive</p>
    </div>
    <div class="panel panel3">
      <p>Experience</p>
      <p>It</p>
      <p>Today</p>
    </div>
    <div class="panel panel4">
      <p>Give</p>
      <p>All</p>
      <p>You can</p>
    </div>
    <div class="panel panel5">
      <p>Life</p>
      <p>In</p>
      <p>Motion</p>
    </div>
  </div>

</body>
</html>

Respostas

7 CertainPerformance Oct 31 2020 at 22:02

Parece muito bom para mim. Só consigo ver algumas coisas a considerar:

Verificação propertyName mais preciso Você tem if(e.propertyName.includes('flex')) {porque usos Safari flexe outros usam flex-grow. Tem certeza de que a flexsubstring não estará presente em nenhuma outra transição CSS possível? Mesmo se você tiver certeza, os leitores do código terão certeza? Eu mudaria para um ===teste contra ambas as possibilidades, ou pelo menos usaria startsWith(o que é um pouco mais apropriado do que .includesaqui, já que ambas as possibilidades começam com flex).

Você também pode mover o comentário sobre o nome do evento de transição para o JS e também para o CSS.

Configuração concisa de classList Quando você deseja adicionar um nome de classe ou remover um nome de classe, com base em uma condição, pode condensar um if(...) classList.add(...) else(...) classList.removeem um único classList.togglecom um segundo argumento que indica se deseja adicionar ou remover a classe. Seu

if(panel.classList.contains('open')) {
  panel.classList.add('open-active');
} else {
  panel.classList.remove('open-active');
}

simplifica para

const { classList } = panel;
classList.toggle('open-active', classList.contains('open'));

Compatibilidade do navegador Porém, alguns navegadores antigos não suportam o segundo argumento, então considere que tipo de navegadores você precisa oferecer suporte. Se você deseja oferecer suporte apenas a navegadores razoavelmente atualizados, tudo bem. Outra coisa a ter em mente é que NodeList.prototype.forEachsó foi introduzido há alguns anos, por volta de 2016 ou 2017 IIRC; como startsWith, é mais recente que ES6, então use um polyfill ou use iteradores e Babel, por exemplo:

for (const panel of panels) {
  // do stuff with panel

(se você quiser suportar o IE, você deve usar o Babel de qualquer maneira, para transpilar seu código para a sintaxe ES5)

Retorno sem efeito? panels.forEach(panel => panel.addEventListenerretorna o valor da chamada addEventListenerpara o chamador de forEach. Uma vez forEachque não olha o que seus retornos de chamada retornam, isso não faz nada. Não é um problema real, mas alguns podem considerar que o código faz um pouco mais de sentido se o forEachretorno de chamada retornar void (sem returninstrução ou retorno implícito). (Descrito no TSLint do TypeScript aqui )

Painéis clicáveis Como os painéis são clicáveis, talvez você altere do cursor padrão para cursor: pointerpara tornar mais óbvio para o usuário que eles devem ser clicados?

Eu mudaria .panel>*para o espaço entre os elementos nos seletores.panel > * - fica um pouco mais fácil de ler quando os elementos separados são separados por espaços.

Painéis repetitivos em vez de

<div class="panel panel1">
</div>
<div class="panel panel2">
</div>

.panel1 {
  background-image: url(https://source.unsplash.com/gYl-UtwNg_I/1500x1500);
}

.panel2 {
  background-image: url(https://source.unsplash.com/rFKUFzjPYiQ/1500x1500);
}

Considere usar em :nth-childvez disso, permitindo que você remova as panel#classes extras inteiramente.

.panel:nth-child(1) {
  background-image: url(https://source.unsplash.com/gYl-UtwNg_I/1500x1500);
}

.panel:nth-child(2) {
  background-image: url(https://source.unsplash.com/rFKUFzjPYiQ/1500x1500);
}

const panels = document.querySelectorAll('.panel');

panels.forEach((panel) => {
  panel.addEventListener('click', () => {
    const isOpen = panel.classList.contains('open');
    panels.forEach(panel => panel.classList.remove('open'));
    if (!isOpen) {
      panel.classList.add('open');
    }
  });
});

panels.forEach((panel) => {
  panel.addEventListener('transitionend', e => {
    /* Safari transitionend event.propertyName === flex */
    /* Chrome + FF transitionend event.propertyName === flex-grow */
    if (e.propertyName === 'flex' || e.propertyName === 'flex-grow') {
      panels.forEach(panel => {
        const { classList } = panel;
        classList.toggle('open-active', classList.contains('open'));
      });
    }
  })
});

html {
  box-sizing: border-box;
  background: #ffc600;
  font-family: 'helvetica neue';
  font-size: 20px;
  font-weight: 200;
}

body {
  margin: 0;
}

*,
*:before,
*:after {
  box-sizing: inherit;
}

.panels {
  min-height: 100vh;
  overflow: hidden;
  display: flex;
}

.panel {
  background: #6B0F9C;
  box-shadow: inset 0 0 0 5px rgba(255, 255, 255, 0.1);
  color: white;
  text-align: center;
  align-items: center;
  /* Safari transitionend event.propertyName === flex */
  /* Chrome + FF transitionend event.propertyName === flex-grow */
  transition:
    font-size 0.7s cubic-bezier(0.61, -0.19, 0.7, -0.11),
    flex 0.7s cubic-bezier(0.61, -0.19, 0.7, -0.11), background 0.2s;
  font-size: 20px;
  background-size: cover;
  background-position: center;

  flex: 1;
  display: flex;
  justify-content: center;
  flex-direction: column;
  
  cursor: pointer;
}

.panel:nth-child(1) {
  background-image: url(https://source.unsplash.com/gYl-UtwNg_I/1500x1500);
}

.panel:nth-child(2) {
  background-image: url(https://source.unsplash.com/rFKUFzjPYiQ/1500x1500);
}

.panel:nth-child(3) {
  background-image: url(https://images.unsplash.com/photo-1465188162913-8fb5709d6d57?ixlib=rb-0.3.5&q=80&fm=jpg&crop=faces&cs=tinysrgb&w=1500&h=1500&fit=crop&s=967e8a713a4e395260793fc8c802901d);
}

.panel:nth-child(4) {
  background-image: url(https://source.unsplash.com/ITjiVXcwVng/1500x1500);
}

.panel:nth-child(5) {
  background-image: url(https://source.unsplash.com/3MNzGlQM7qs/1500x1500);
}

/* Flex Children */
.panel > * {
  margin: 0;
  width: 100%;
  transition: transform 0.5s;
  flex: 1 0 auto;
  display: flex;
  align-items: center;
  justify-content: center;
}

.panel *:first-child {
  transform: translateY(-100%);
}

.panel *:last-child {
  transform: translateY(100%);
}

.panel.open-active *:first-child,
.panel.open-active *:last-child {
  transform: translateY(0);
}

.panel p {
  text-transform: uppercase;
  font-family: 'Amatic SC', cursive;
  text-shadow: 0 0 4px rgba(0, 0, 0, 0.72), 0 0 14px rgba(0, 0, 0, 0.45);
  font-size: 2em;
}

.panel p:nth-child(2) {
  font-size: 4em;
}

.panel.open {
  font-size: 40px;
  flex: 5;
}

<link href="https://fonts.googleapis.com/css?family=Amatic+SC" rel="stylesheet" type="text/css">
<div class="panels">
  <div class="panel">
    <p>Hey</p>
    <p>Let's</p>
    <p>Dance</p>
  </div>
  <div class="panel">
    <p>Give</p>
    <p>Take</p>
    <p>Receive</p>
  </div>
  <div class="panel">
    <p>Experience</p>
    <p>It</p>
    <p>Today</p>
  </div>
  <div class="panel">
    <p>Give</p>
    <p>All</p>
    <p>You can</p>
  </div>
  <div class="panel">
    <p>Life</p>
    <p>In</p>
    <p>Motion</p>
  </div>
</div>

4 SᴀᴍOnᴇᴌᴀ Nov 01 2020 at 05:50

Reveja

Eu concordo com a resposta de CertainPerformance: o código parece muito bom. A indentação parece consistente, os nomes das variáveis são apropriados e as linhas bem terminadas. A legibilidade é ótima.

Loops ineficientes

O código nos manipuladores de eventos percorre todos os elementos do painel, mas no máximo apenas dois elementos teriam atualizações em sua lista de classes. Veja a sugestão abaixo sobre formas de eliminar os loops.

Excesso de regras CSS

O estilo font-size: 20px;não é necessário, .panelpois a mesma regra também é especificada em html, além de ser substituída por seletores mais específicos.

Sugestão

Remova os loops usando a delegação de eventos

Em vez de adicionar ouvintes de evento a cada elemento do painel, ouvintes de evento podem ser adicionados ao elemento de contêiner. Isso exigiria alterar os manipuladores de eventos para ver o destino do evento e determinar se o destino correspondia a um painel ou filho de um painel - pode ser feito com o .closest()método. E um HTMLCollection ao vivo de elementos com o nome da classe openpode ser obtido uma vez usando document.getElementsByClassName('open');. Então, se algum elemento tiver essa classe quando o manipulador de cliques for chamado, o nome da classe poderá ser removido.

Isso permitiria adicionar e remover painéis sem a necessidade de registrar os manipuladores de eventos neles. Embora possa não fazer uma diferença perceptível em uma página pequena como esta, é aconselhável considerar os locais onde um loop pode ser evitado.