Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alteracoes, grupo 5 #1

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Conversation

giovaner10
Copy link

@giovaner10 giovaner10 commented Mar 27, 2022

Alterações na estrutura do projeto, integrando a responsividade e a utilização de rotas

@renato3x renato3x added the bug Something isn't working label Mar 28, 2022
Copy link
Owner

@renato3x renato3x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Há muitas incongruências no seu código, como falta de organização no desenvolvimento e também vocês fugiram do escopo de que isso é um projeto pessoal.

@@ -0,0 +1 @@
<p>a pagina de perfil vai ficar bem aqui</p>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qual o sentido de criar uma nova página e não implementá-la?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ela ainda esta em construção , só reservamos o espaço para a sua implantação.

@@ -1,7 +1,7 @@
<nav id="navbar">
<div class="img-logo">
<a href="#">
<img [src]="instagramLogoSource" alt="Instagram - Logo">
<img [src]="instagramLogoSource" alt="Instagram - Logo" routerLink="">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qual o motivo de colocar um routerLink vazio e ainda dentro de um elemento de imagem?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No elemento externo a imagem, o link não estava funcionando, o vazio se dar para desviar para rota de home, deixei uma path vazia, mas antes estava "path: 'home'", acabei esquecendo de remover. De toda forma era desnecessário, ja que o "href='#'", ia direcionar para o home

@@ -1,3 +1,13 @@
@media(max-width: 900px){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

código da responsividade mal organizado

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vou concertar, irei criar uma folha de estilo exclusiva para as mesmas.

@@ -2,8 +2,8 @@
<div class="user-infos">
<img [src]="userImageSource" alt="access my github :) https://github.com/renato3x" class="user-img">
<div class="user-name-infos">
<span class="user-username">Renato3x</span>
<span class="user-name">Renato</span>
<span class="user-username">SoulCodeAcademy</span>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isso continua sendo um projeto pessoal, de portifólio. Alterar isso não é aceito

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Desculpa professor, não sabia, achei que íamos somente alterar e apresentar.


{
path: 'soulcodeacademy',
component: PerfilComponent
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pelo que vi, esse componente PerfilComponent é um componente que é uma página. Neste projeto, há uma pasta exclusivamente para isso. Criar em outra pasta, afeta a organização do código. Além disso, note que foi utilizado o padrão inglês no desenolvimento. Fugir dessa organização pode ser ruim

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tudo bem professor, isso se deu porque ainda não mexemos nessa parte da nova pagina, já que a responsabildade da criação dela era de outros integrantes do grupo, e ela ainda esta em desenvolvimento.

@renato3x
Copy link
Owner

Notei que vocês cometeram um bug na parte dos stories: as alterações que vocês fizeram, fizeram com que não fosse mais possível navegar entre os stories, ou seja, não consigo mais fazer a rolagem entre os stories.

@giovaner10
Copy link
Author

Professor, em questão ao problema da rolagem entre os stories, eu acabei de pegar no projeto original, e também não há rolagem entre os stories, clico e não acontece nada.

tikaoo and others added 2 commits April 2, 2022 20:42
@tikaoo
Copy link

tikaoo commented Apr 4, 2022

Professor, incluí rotas "filhas" para aplicação do lazy loading está correto dessa forma? Lembrando que ainda falta a elaboração de uma página de perfil que será criada.

@renato3x renato3x removed the bug Something isn't working label Apr 5, 2022

const routes: Routes = [
{
path:'aside-data',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qual a necessidade de criar rotas para mostrar um componente de cada vez?

import { RouterModule, Routes } from '@angular/router';
import { PerfilComponent } from '../components/perfil/perfil.component';

const routes: Routes = [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Por qual motivo fazer um módulo de roteamento unicamente para a página de perfil? A aplicação é simples, considero que não seria necessário fazer isso. Além disso, ainda notei que a página de perfil ainda está no lugar errado. Se o componente Perfil é uma página, ele deveria estar na pasta /pages, junto com o componente Home. E também ele ainda não segue o padrão inglês


{
path: 'soulcodeacademy',
loadChildren: ()=> import('./components/perfil/perfil.module').then(m=> m.PerfilModule)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

É uma boa ideia a do Lazy Loading. Mas não estou vendo necessidade de utilizá-lo nesse projeto

@renato3x
Copy link
Owner

renato3x commented Apr 5, 2022

Vocês deixaram os módulos adicionais que vocês criaram totalmente desorganizados. O dois módulos do módulo component (o principal e o routing) estão em lugares diferentes. Isso, na organização de um projeto Angular real, é totalmente errado. Aconselho a desfazerem estas últimas alerações

…ava muito para o carregamento, pois estaticamente ja carregava 50 publicações, com a adoção do scrool, ela carrega estaticamente 5 posts, e dinaminicamente ela carrega os demais a mediade que rolamos a pagina. Usei a ngx-infinite-scroll para esse implementação
@renato3x
Copy link
Owner

Por favor, me informe o nome de todos os integrantes

alvaroaxsmith and others added 5 commits May 5, 2022 12:00
…mist-1.2.6

Bump minimist from 1.2.5 to 1.2.6
Bumps [eventsource](https://github.com/EventSource/eventsource) from 1.1.0 to 1.1.1.
- [Release notes](https://github.com/EventSource/eventsource/releases)
- [Changelog](https://github.com/EventSource/eventsource/blob/master/HISTORY.md)
- [Commits](EventSource/eventsource@v1.1.0...v1.1.1)

---
updated-dependencies:
- dependency-name: eventsource
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4.
- [Release notes](https://github.com/caolan/async/releases)
- [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md)
- [Commits](caolan/async@v2.6.3...v2.6.4)

---
updated-dependencies:
- dependency-name: async
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…tsource-1.1.1

Bump eventsource from 1.1.0 to 1.1.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants