redu / apps

Redu's application repository
2 stars 2 forks source link

Melhoria do colapsar respostas #83

Closed filipewl closed 11 years ago

filipewl commented 11 years ago

Avaliar a possiblidade de fazer o colapsar respostas somente por client-side. Atualmente ele é feito server-side e se faz necessário o uso de muita lógica na view.

embs commented 11 years ago

@julianalucena e @filipewl, se bem me lembro, o intuito deste issue é transformar o carregamento de respostas a comentários (que é feito através de uma requisição assíncrona) de maneira que ele seja feito de uma só vez (isto é, carregar todas as respostas em conjunto com os comentários ao renderizar um aplicativo). Isto não poderia causar um atraso desnecessário na realização da requisição ao carregar um aplicativo com comentários que possuem muitas respostas? Além disso, discuti com @fltiago a respeito e detetamos que o Facebook realiza esse tipo de carregamento através de requisições assíncronas também – com ganho no desempenho da resposta à requisição.

@filipewl, podes me lembrar onde é que está o excesso de lógica nas views que motivou a criação do issue? Dei uma olhada aqui e a parte que abrange o carregamento de respostas parece bem enxuta.

Quero formalizar esta discussão (nesta thread) para decidirmos se a modificação do código deve ou não ser feita.

julianalucena commented 11 years ago

@embs, entendo o ganho de performance, mas na época teria sido mais simples de implementar o colapsar apenas com client-side. De qualquer forma, a lógica na view que nós comentamos podem ser vistas nestes arquivos, por exemplo. São ifs utilizados puramente para estilo, porém concordo que isso passa a ser aceitável se houver ganho de performance.

Porém, identifiquei que a paginação vai para Apps#show e carrega tudo novamente quando apenas deveria carregar os comentários. Outro ponto é que se isso será um padrão, é interessante que essa lógica da view se concentre em um lugar, de modo que um helper possa ser chamado, por exemplo, para mostrar as respostas (utilizando-se da view). Também identifiquei que o limite de respostas a serem mostrados está hard coded, sugiro que isto fique como uma configuração da aplicação.

embs commented 11 years ago

@julianalucena, pelo que entendi do código, esses ifs utilizados para estilo seriam necessários independentemente de o carregamento ocorrer de uma só vez ou com uma requisição assíncrona – já que eles servem para adicionar (ou não) aqueles elementos gráficos para comentários que possuem muitas respostas.

Acho que você confundiu o carregamento de comentários – que realmente vai pra Apps#show – com o carregamento de respostas – que vai pra Comments#show.

Não entendi a parte de aplicar um padrão de carregamento através de helpers. Isso está no escopo desta issue?

Por fim, modifiquei a forma como é definida a quantidade de comentários exibidos por página – que agora é uma configuração – e comitei diretamente no branch master.

embs commented 11 years ago

@julianalucena, criei a issue #174 pra deixar o negócio mais consistente.

julianalucena commented 11 years ago

Eu sei, @embs. Mas eu me referi aos ifs para indicar qual era a lógica da view referenciada por @filipewl.

Ué, mas a listagem de comentários vai pra Comments#show? Este action deveria ser usado para visualizar apenas um comentário. Não?

Tudo bem, podemos deixar para quando uma paginação parecida seja necessária.

Faltou tu configurar a paginação de resposta.

embs commented 11 years ago

Tens razão, @julianalucena. A URL mais adequada para carregar todas as respostas de um comentário é a que vai para CommentsController#index e carrega todos os comentários (respostas) associados a um comentário, correto? (de acordo com a configuração do nosso routes).

Criei a configuração da paginação!

julianalucena commented 11 years ago

Na realidade, a url para carregar várias respostas, deve ser o index de resposta.