JavaScript - How to improve this code?

4

As there is no Code Review Stack Exchange in English and I ask for help in one I'm working on: suggestions and criticisms about a chat script that I'm developing in JavaScript to streamline an existing one in PHP.

This idea has become a challenge for me because I can not change the source code of PHP since the script is a suggestion of improvement to a site that I use daily and the person in charge of it does not have time at the moment, nor made the code available.

The current script works by generating a table where each message sent consists of two <tr> , the first one with three <td> the data of who sent and the second with a single <td> with the message. The sending of messages is by a POST request, with the message in the mensagem field and they are deleted with a GET request with the message ID to be deleted in the del field.

To dynamize it, I decided to implement a function that compares the current page with a new loaded via AJAX. The cases where messages are added and removed are considered, in addition there is the notice of new messages per message at the top of the page and in the title of the page. I used jQuery because the library is already used on other pages of the site.
The current version of the code is now in this gist , and here too:

// ==UserScript==
// @name        Sugestões de Mudança para a Shoutbox
// @version     1.5.14
// @grant       none
// @updateURL   https://rawgit.com/qgustavor/fc66dc1aa54c2c3a9970/raw/shoutbox.js
// @downloadURL https://rawgit.com/qgustavor/fc66dc1aa54c2c3a9970/raw/shoutbox.js
// ==/UserScript==

;(function functionWrapper(){
  // Coloque 'true' para voltar a forma antiga de apagar mensagens:
  var APAGAR_USANDO_CONFIRM = false;
  
  // Para testar só colar o código no console
  // (aperte F12 e ESC que deve abrir)
  // Se for bloqueado siga as instruções do navegador
  
  // Caso queira testar esse script como user script
  // as linhas abaixam lidam com a adaptação.
  if (typeof $ === 'undefined') {
    var el = document.createElement('script');
    el.innerHTML = '(' + functionWrapper  + '());';
    document.head.appendChild(el);
    return;
  }
  
  // Se quiser aplicar as mudanças no site
  // leia os comentários que explicam
  // o que o script altera.

  // Desative o script atual de atualização
  // As linhas abaixo fazem isso, porém
  // não é a melhor maneira e sim removendo.
  $.fn.load = $.noop;

  // Adicione esse código CSS em um arquivo 
  // O código da animação veio do css-spinners.com
  // então processado no AutoPrefixer e finalmente Devilo.us
  $('<style>', {
    html: '@-webkit-keyframes spinner-loader{0%{-webkit-transform:rotate(0);transform:rotate(0)}100%{-webkit-transform:rotate(360deg);transform:rotate(360deg)}}@keyframes spinner-loader{0%{-webkit-transform:rotate(0);transform:rotate(0)}100%{-webkit-transform:rotate(360deg);transform:rotate(360deg)}}.spinning{-webkit-animation:spinner-loader 1500ms infinite linear;animation:spinner-loader 1500ms infinite linear}.message-alert{position:absolute;text-align:center;background:#bbdefb ;left:0;top:0;right:0;padding:1em}.shoutbox_contain{height:245px/* valor padrão para navegadores antigos */;height:calc(100vh - 45px)}@-webkit-keyframes new-message{0%{background:#bbdefb}100%{background:#f4f4f4!important}}@keyframes new-message{0%{background:#bbdefb}100%{background:#f4f4f4!important}}.highlight{-webkit-animation:new-message 3s ease-in 1;animation:new-message 3s ease-in 1}.history-link-row{text-align:center;background:#f4f4f4}.history-link-row td{padding:1em}.shoutboxform_input{margin-left:10px;margin-right:150px}.shoutboxform_input input{width:100%}.shoutboxform_links{float:right;width:100px;text-align:center;padding:6px 0}.shoutboxform_links a{margin:0 5px}td:only-child a{word-break:break-all}form{margin-top:5px}'
  }).appendTo('head');
  
  // Substitua o formulário baseado em tabelas com um baseado em classes:
  $('form').html('<div class="shoutboxform_links"><a href="#" class="emoticon-link"><i title="Emoticons" class="icon-heart"></i></small></a><a href="?"><i title="Atualizar" class="icon-refresh"></i></a><a href="shoutbox.php?history=1" target="_top"><i title="Histórico" class="icon-book"></i></a></div><div class="shoutboxform_input input-append"><input class="shoutbox_msgbox" placeholder="→ CAMPO DE CHAT ←" type="text" name="message"><button class="btn" type="submit" name="submit">Enviar</button></div>');
  
  var DEFAULT_TITLE = top.document.title;
  var container = $('.shoutbox_contain');
  var containerBody = $('.shoutbox_contain tbody');
  var refreshIcon = $('.icon-refresh').click(loadAjax);
  var unreadMessages = $();
  var timeBetweenReloads;
  var timer;
  
  // Retorna a função de emoticons:
  $('.emoticon-link').on('click', function(evt) {
    evt.preventDefault();
    PopMoreSmiles('shoutboxform', 'message');
  });
  
  // Adiciona um link para ver o histórico de mensagens no final
  // Se for adicionado no HTML é melhor
  containerBody.append('<tr><td></td><td></td><td></td><tr>'); // As <tr> são sempre aos pares
  $('<tr>', {'class': 'history-link-row'}).appendTo(containerBody)
  .append($('<td>',{colspan:3}).append(
    $('<a>', {
    href: 'shoutbox.php?history=1',
    target: '_top', // _blank = nova janela, _top = na janela atual
    text: 'Ver histórico de mensagens' // ou 'Ver mensagens anteriores'
  })));

  setRefreshTimer();
  setMessageDeleteHandlers();
  
  function setMessageDeleteHandlers() {
    $('a[href*="shoutbox.php?del="][onclick]')
    .removeAttr('onclick')
    .on('click', handleMessageDelete);
  }
  
  function handleMessageDelete(evt) {
    var $this = $(this);
    var deleteHref = $this.attr('href').match(/\?(del=\d+)/)[1];
    
    evt.preventDefault();
    
    // Caso queira voltar a forma antiga só
    // alterar o valor dessa constante
    if (APAGAR_USANDO_CONFIRM) {
      if (confirm('Tem certeza que quer deletar esta mensagem?')) {
        loadAjax({ data: deleteHref });
      }
      return;
    }
    
    // Ao invés de confirm, precisa de alguns ajustes...
    showMessage('Você tem certeza? Clique nessa mensagem para confirmar', function() {      
      loadAjax({
        data: deleteHref
      });
    });
  }
  
  $('form').on('submit', handleMessageSent);
  
  function handleMessageSent(evt) {
    // Por praticidade (i.e. não ter que lembrar da documentação)
    // copiei isso do http://ginpen.com/2013/05/07/jquery-ajax-form/
    var $form = $(this),
        $input = $form.find('.shoutbox_msgbox');
        
    evt.preventDefault();
    
    loadAjax({
      url: $form.attr('action'),
      type: $form.attr('method'),
      data: $form.serialize()
    })
    .always(function () {
      $input.removeAttr('disabled').val('');
    });
    
    $input.attr('disabled', true).val('Enviando mensagem...');
  }

  // As duas funções abaixo chamam uma a outra
  // para que seja possível controlar o timer
  // e assim pará-lo quando clicar no .icon-refresh
  function refreshLoop() {
    loadAjax(setRefreshTimer);
  }
  
  function setRefreshTimer() {
    // Usar valor padrão de 30, caso não tenha sido definido
    timer = setTimeout(refreshLoop, timeBetweenReloads || 30e3);
  }

  // Carregar as novas mensagens usando $.ajax
  // Além disso adicionar uma animação de carregando
  // e parar o timer, evitando que a página
  // seja carregada várias vezes seguidas
  function loadAjax(opt) {
    // Se a função for chamada por um evento:
    if (opt instanceof $.Event) {
      opt.preventDefault();
      opt = {};
    }
    
    refreshIcon.addClass('spinning');
    clearTimeout(timer);
    
    return $.ajax('shoutbox.php', opt || {})
      .done(gotNewMessages)
      .fail(failedGettingMessages)
      .always(afterChatLoaded);
  }
  
  // Independente se carrregou ou não
  // parar a animação e agendar para carregar novamente
  function afterChatLoaded() {
    refreshIcon.removeClass('spinning');
    setRefreshTimer();
  }
  
  // Avisar caso as mensagens não carreguem e voltar
  // o tempo entre recarregamentos ao padrão
  function failedGettingMessages() {
    showMessage('Não foi possível carregar novas mensagens');
    timeBetweenReloads = null;
  }

  // Caso a página tenha sido carregada então processá-la
  function gotNewMessages(returnedHtml) {
    // Amarzena estado anterior de scroll antes de aplicar as modificações:
    var firstOldMessage = containerBody.children().first();
    var oldPosition = firstOldMessage.offset().top;
    var scrolledLength = container.scrollTop();
    
    // Tentar adivinhar quantas mensagens são novas
    // comparando a página atual com a carregada
    var currentMessages = $('tr td:only-child');
    var currentMessagesData = currentMessages.map(function (){
      return $(this).html();
    }).get();
    var returnedMessages = $('<div>', {html: returnedHtml})
    .find('.shoutbox_contain tr td:only-child');
    
    var lastIndex = -1;
    var newMessages = $();
    
    returnedMessages.each(function () {
      var $this = $(this);
      var elementData = $this.html();
      // O segundo argumento do indexOf garante a ordem das mensagens:
      var index = currentMessagesData.indexOf(elementData, lastIndex + 1);
      
      // A mensagem já tinha sido registrada:
      if (index !== -1) {
        // Elimina mensagens apagadas:
        if(index - lastIndex > 1) {
          currentMessages.slice(lastIndex + 1, index).each(function (n) {
            // Mais informação sobre essa corrente nas linhas abaixo
            var removedElements = $(this).parent().prev().addBack();
            
            // Remover da lista de mensagens não lidas:
            // Adicionado porque o número de mensagens estava alto
            // Removido por que ficou baixo. Motivo: desconhecido.
            // unreadMessages.find(removedElements).remove();
            
            // Remover da lista de mensagens mostradas:
            removedElements.fadeOut(1200, function(){
              $(this).remove();
            });
          });
        }
        lastIndex = index;
        
      // A mensagem não tinha sido
      // registrada então adicioná-la
      } else {
        // $this é o <td> no segundo <tr>
        // logo para pegar toda a mensagem
        // seleciona o <tr> e o anterior a ele.
        var newMessagesElements = $this.parent().prev().addBack();
        currentMessages.eq(lastIndex + 1).parent().prev().before(newMessagesElements);
        
        // Só considerar mensagens novas aquelas que forem adicionadas anteriormente as já existentes:
        // Por algum motivo está considerando menos mensagens do que deveria
        if (lastIndex + 1 <= newMessages.length) {
          newMessages = newMessages.add(newMessagesElements.prevAll().andSelf());
        }
      }
    });
    
    // Se estiver houvendo uma conversa ativa então
    // carregar mensagens em um intervalo mais rápido
    // se não aumentar o intervalo para aliviar o servidor
    // ajuste os valores se necessário
    timeBetweenReloads = Math.max(
      10, // Intervalo mínimo de 10 segundos
      60 - newMessages.length * 15 // Cada nova mensagem reduz o intervalo em 15 segundos
    ) * 1000; // Finalmente converte para milisegundos

    if (newMessages.length === 0) { return; }
    setMessageDeleteHandlers();
    
    // Se não estiver na página então deixar
    // um aviso pelo título da página
    if (document.hidden) {
      // unreadMessages amarzena os elementos das novas mensagens
      unreadMessages = unreadMessages.add(newMessages);
      // Como cada mensagem é composta de dois elementos logo se divide por dois
      top.document.title = '(' + (unreadMessages.length / 2) + ') ' + DEFAULT_TITLE;
      
      // Se for a primeira vez que isso acontece:
      if (unreadMessages.length === newMessages.length) {
        // O código para detectar quando voltou a página é simplificado,
        // a versão completa dele está nessa página:
        // https://developer.mozilla.org/en-US/docs/Web/Guide/User_experience/Using_the_Page_Visibility_API
        $(document).one('visibilitychange', function () {
          // volta o título ao padrão:
          top.document.title = DEFAULT_TITLE;
          // Seleciona a ultima mensagem e alinha o fundo do scroll com ela:
          scrollToElement(unreadMessages.last(), function () {
            unreadMessages.addClass('highlight');
            // Limpa a fila de mensagens não lidas:
            unreadMessages = $();
          });
        });
      }
    // Se estiver lendo mensagens anteriores
    } else if (scrolledLength !== 0) {
      // Manter o scroll no mesmo ponto,
      // para não interromper a leitura:
      container.scrollTop(scrolledLength + firstOldMessage.offset().top - oldPosition);
      
      // Avisar que há novas mensagens por
      // meio de um aviso no topo da página
      showMessage(newMessages.length === 2 ? 'Há uma mensagem nova' :
        ('Há ' + (newMessages.length / 2) + ' novas mensagens'), function () {
          scrollToElement(newMessages.last(), function () {
            newMessages.addClass('highlight');
          });
        });
    } else {
      newMessages.addClass('highlight');
    }
  }
  
  // Cria uma mensagem de aviso no topo da página
  // Aceita uma função no segundo argumento
  // que é chamada caso a mensagem seja clicada
  function showMessage(message, callback) {
    var el = $('<div>', {
      text: message,
      'class': 'message-alert'
    })
    .appendTo('body')
    .on('mousedown touchstart', function handler(evt) {
      // Evita ser chamado mais de uma vez
      el.off('mousedown touchstart', handler);
      
      // Chama o callback, se definido
      if (typeof callback == 'function') {
        callback();
      }
      
      // Faz a animação de fadeout
      el.stop(true).fadeOut(300, function () {
        el.remove();
      });
    })
    // Usando as animações do jQuery
    // Há CSS3, mas nesse caso tudo bem
    .hide().fadeIn(1000)
    .delay(5e3 /* 5 segundos*/ )
    .fadeOut(1000, function () {
      el.remove();
    });
  }
  
  // Coloca o fundo do scroll no elemento
  // Se não for possível irá o mais próximo
  function scrollToElement(el, callback) {
    container.animate({
      scrollTop: el.position().top - container.height() + container.scrollTop() + el.height()
    }, 600, 'swing', callback || $.noop);
  }
}());                    
asked by anonymous 31.08.2015 / 23:13

1 answer

2

An interesting challenge, but unfortunately I'm running out of time to analyze all the code in detail and propose improvements.

Beware of competition

In general, when random problems occur, such as these duplicate or disappear messages, the problem is related to the competition, so in simple tests this does not happen, but when multiple users use the system errors appear.

One way to better determine the problem is to add some kind of log into production. When someone complains, review the log and see what messages were displayed and when. This way you can easily isolate the problem.

Beware of the Network

Another possible scenario is that a bad connection causes some Ajax calls to be lost and some duplicates. This is another case where there are no problems in the tests, but they appear in the clients.

Assuming there is no silly or logical error in the code, my suggestion is to use some value that can identify each message individually rather than relying on the order and quantity of messages and trying to guess is in the code) what are the new messages.

Move away from HTML

Another possible improvement would be to decrease the amount of code that manipulates and directly selects HTML.

Maybe using a library like AngularJS or ReactJS is a bit easier in that case, because then you can only worry about getting the data and leave it to the library to show the messages correctly.

Considerations

Anyway, I know they were abstract tips and they take some time to implement, but I hope it was some help.

    
01.09.2015 / 08:40