Skip to content

projeto brainn ainda incompleto#63

Open
Gabriel-Goncalves47 wants to merge 1 commit intomasterfrom
projeto-front-end
Open

projeto brainn ainda incompleto#63
Gabriel-Goncalves47 wants to merge 1 commit intomasterfrom
projeto-front-end

Conversation

@Gabriel-Goncalves47
Copy link
Collaborator

POR FAVOR, EDITE ESSA MENSAGEM INSERINDO AS INFORMAÇÕES DO SEU PROJETO. COMECE APAGANDO ESSA LINHA.

INSIRA O NOME NOME DO PROJETO AQUI

O que funciona

  • DESCREVA O QUE FUNCIONA NO SEU PROJETO

O que não funciona

  • DESCREVA O QUE NÃO FUNCIONA NO SEU PROJETO

Link Surge

A PARTIR DA SEMANA 5, INSIRA AQUI O LINK DO SURGE. ANTES DISSO, APAGUE ESSAS DUAS LINHAS.

Imagens

TIRE PRINTS DAS TELAS DO SEU SITE E COLE AQUI

@Janaylla
Copy link

Janaylla commented Jan 27, 2022

Parabéns pela entrega!

Requisitos do projeto ✅

Implementações Feito
A aplicação terá que suportar 6 sorteios: Mega-sena, Quina, Lotofácil, Lotomania, Timemania e Dia de sorte
Seguiu o Layout padrão e seus temas
Ao mudar esse combo-box, terá que mudar o tema do sorteio, número do sorteio, data do sorteio e números sorteados.
A aplicação terá que ser responsiva, pelo menos para celulares, conforme layout.
Criar rotas com React Router DOM (opcional).
Criou um Readme com descrição do projeto. -
Disponibilizou o link do deploy -
Feedback do código Feito
React com TypeScript -
Testes com React Testing Library e/ou Cypress -
O consumo da API pode ser feito via REST ou GraphQL.
Todos esses sorteios estarão em um combo-box/select.

Comentários da pessoa avaliadora

Parabéns pela entrega. Vi que faltava algumas paginas ficarem prontas. Aqui vão alguns ponto positivos do seu código:
Separação de responsabilidades
Uso do GlobalState
Base URL isolada
Padrão de cores como o descrito no figma

Mas, há alguns pontos de atenção que podem ser melhorados:
O principal é: todas as paginas são iguais pela descrição do figma e o React traz para a gente a possibilidade de reaproveitar muito código nesse tipo de situação.
Dica: Uma pagina apenas que recebesse por parâmetro(pela url da pagina) o nome do concurso teria resolvido o seu problema melhor do que criar varias paginas diferentes. E as cores poderiam ser mudadas passando props para o styled-component/trocando o className dos elementos.
Tinha muita mistura de línguas e os nomes da variáveis nem sempre foram bem escolhidos, isso deixou o código confuso.

Alguns comentários ao longo do código abaixo:

<option value="megasena">Mega-Sena</option>
<option value="quina">Quina</option>
<option value="timemania">Timemania</option>
</select>

Choose a reason for hiding this comment

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

Aqui você fez um select com os dados mocados, o ideal é que você utilizasse os dados que você guardou globalmente em ids, onde você tem um array desse tipo:

[
{
"id": 1,
"nome": "Mega Sena"
}
]

E poderia fazer um map com ele,

Choose a reason for hiding this comment

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

Nessa parte do código também o valor de identificação é o id, seria melhor ter trabalhado com ele.


const GlobalState = (props) => {

const [loto, setLoto] = useState("")

Choose a reason for hiding this comment

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

não precisava deste estado, pois você também coloca um "identificador" na url

alert(error.message)
})
}

Choose a reason for hiding this comment

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

o useEffext que chama a getSortitions e a getLoteries poderia estar no estado global, já que todas as paginas precisam assim que roda o site, e não muda.
Os dados que variam são os que vem do GET: /concursos/{id}, já que este depende do id do concurso.

Choose a reason for hiding this comment

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

Senti falta também de uma função que faz o find para encontrar o código do concurso, já que esse tipo de informação todas as paginas iriam precisar filtrar,

default:
goToMegaSena(history)
}
}

Choose a reason for hiding this comment

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

Apenas o select utiliza esta função, poderia estar no component em vez do estado global.

DiaDeSorte Page
<Select />
</div>
)

Choose a reason for hiding this comment

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

Passando a logica para uma única pagina você iria evitar muito retrabalho e todas os concursos seriam mostrados,

<div className='sortNumber' id="lotofacilSortNumber4">{numeros && numeros[3]}</div>
<div className='sortNumber' id="lotofacilSortNumber5">{numeros && numeros[4]}</div>
<div className='sortNumber' id="lotofacilSortNumber6">{numeros && numeros[4]}</div>

Choose a reason for hiding this comment

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

Quando você tiver um array e quer exibi-lo, um map é a melhor solução, se você não o fez por conta do id, eu deixo a minha sugestão de como você poderia escrever o map e deixar o id dinâmico, E evitaria problemas como o das linhas 69 2 79 em que você repetiu o índice 4.

{
    numeros && numeros.map((numero, index) => {
        return <div className='sortNumber'
            id={`lotofacilSortNumber${index + 1}`}>{numero}</div>
    }
    )
}

@@ -0,0 +1,11 @@
export const setDateToDDMMYYY = (stringDate) => {

Choose a reason for hiding this comment

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

Fez bem em extrair essa logica para uma função

@Janaylla Janaylla requested review from Janaylla January 27, 2022 19:01
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.

2 participants