Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Criado MVP #37

Merged
merged 30 commits into from
Aug 26, 2021
Merged

Conversation

filipebsmaia
Copy link
Contributor

@filipebsmaia filipebsmaia commented Jun 24, 2021

Esse PR contém as seguintes adições/modificações

  • Componentes de Voluntários
  • Componentes de Botão
  • Página Home

Imagens

Mobile utilizando 320px de largura

mobile

Mobile utilizando 375px de largura

375

Tablet utilizando 900px de largura

tablet

Desktop utilizando 1920px de largura

full-page

@filipebsmaia
Copy link
Contributor Author

Algumas ações dos botões e textos não são as ideais/finais, posso modificar eles se poderem me passar o conteúdo a ser posto.

@anapaulagomes
Copy link
Contributor

Caramba, que foda, @filipebsmaia! 😭 ❤️ Vamos ver quem pode revisar.

@anapaulagomes
Copy link
Contributor

@filipebsmaia tentei rodar localmente mas peguei esse erro:

Error: Failed prop type: The prop href expects a string or object in <Link>, but got undefined instead.

Tô usando a versão v14.17.1 do Node. Pode ajudar?

@filipebsmaia
Copy link
Contributor Author

filipebsmaia commented Jun 25, 2021

Oi ana, provavelmente é por conta das variáveis ambiente, vou te enviar a minha aqui
Basca criar um .env.development ou .env.local

NEXT_PUBLIC_SITE_URL=http://localhost:3000
NEXT_PUBLIC_FACEBOOK_URL=https://www.facebook.com/dadosabertosdefeira
NEXT_PUBLIC_INSTAGRAM_URL=https://www.instagram.com/dadosabertosdefeira/
NEXT_PUBLIC_TWITTER_URL=https://twitter.com/DadosDeFeira

Acha que podemos subir pro git as variáveis locais junto com o projeto?

@anapaulagomes
Copy link
Contributor

Show, funcionou! @filipebsmaia. Pode adicionar esse arquivo como exemplo? E atualizar no README também? Eu não sou front então senti falta de saber qual a versão do Node seria compatível. Adicionamos pra facilitar ou não precisa?

Sobre o site: ficou ótimo! Muito foda ver funcionando. hahaha Algumas observações:

  • Podemos remover a busca do canto superior direito por agora
  • Ao clicar em "Preencher formulário" eu pego o erro TypeError: navRef.current is null
  • "Consultar base de dados" pode ser redirecionado para https://www.dadosabertosdefeira.com.br/painel/
  • A logo na barra superior pode ter um link para a home
  • Blog pode redirecionar (abrindo uma nova aba) para dadosabertosdefeira.medium.com
  • Podemos linkar as páginas da pagina superior para as seções né? Atualmente eu clico e vai para uma página separada (tipo em "Colabore")

🏆

@filipebsmaia
Copy link
Contributor Author

Não consegui reproduzir o erro do "Preencher formulário" aqui, consegue me da mais alguma informação sobre ele?

@anapaulagomes
Copy link
Contributor

anapaulagomes commented Jun 25, 2021 via email

@filipebsmaia
Copy link
Contributor Author

filipebsmaia commented Jun 25, 2021

A ação do botão "não encontrou o que queria" mantenho ele redirecionando para a página /busca?

Também só comentei a funcionalidade de busca no Header, visto que futuramente podemos utilizar ela, e como está comentado basta remover o comentário e fazer a implementação.

src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
src/pages/index.js Outdated Show resolved Hide resolved
@anapaulagomes
Copy link
Contributor

A ação do botão "não encontrou o que queria" mantenho ele redirecionando para a página /busca?

Não. O "não encontrou o que queria" tem seu próprio componente (mais detalhes no Figma). Vamos basicamente ensinar um passo a passo de como conseguir informações fazendo um pedido aos órgãos públicos. Podemos remover ou comentar esse botão por agora e implementar em um PR separado. 👍🏽

@anapaulagomes
Copy link
Contributor

O título da página está "Dados abertos de Feira". Podemos colocar o "abertos" em maiúscula? Assim: "Dados Abertos de Feira"

Copy link
Collaborator

@matheusrocha89 matheusrocha89 left a comment

Choose a reason for hiding this comment

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

Deixei alguns comentários relevantes, caso tenha alguma dúvida, só deixar uma mensagem. Ficarei feliz em ajudar

src/components/Button/index.js Outdated Show resolved Hide resolved
src/components/Header/index.js Outdated Show resolved Hide resolved
src/components/VolunteersCard/index.js Outdated Show resolved Hide resolved
src/components/VolunteersCard/styles.module.scss Outdated Show resolved Hide resolved
right: 0px;

@media (max-width: 1280px) {
img {
Copy link
Collaborator

Choose a reason for hiding this comment

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

O mesmo ponto que disse sobre os elementos e as classes 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Então, nesse caso em especifico acredito que esse não há tanto a necessidade, visto que essa a div com classe rightImage só tem o elemento da imagem dentro dela e mais nada, e esse trecho ai é só pra responsividade, porem se necessário posso alterar.

top: 0;
right: 0px;

@media (max-width: 1280px) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seria bom colocarmos esses valores de media em uma variável no arquivo de variables porque provavelmente vamos usar em vários lugares e caso precisemos atualizar algum dia fica mais fácil

src/pages/index.module.scss Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
Co-authored-by: Ana Paula Gomes <1899950+anapaulagomes@users.noreply.github.com>
@netlify
Copy link

netlify bot commented Jul 18, 2021

✔️ Deploy Preview for dadosabertosdefeira ready!

🔨 Explore the source changes: b671cbb

🔍 Inspect the deploy log: https://app.netlify.com/sites/dadosabertosdefeira/deploys/611bc4bbf6f56f000720b685

😎 Browse the preview: https://deploy-preview-37--dadosabertosdefeira.netlify.app

matheusrocha89 and others added 2 commits July 18, 2021 10:10
Co-authored-by: Ana Paula Gomes <1899950+anapaulagomes@users.noreply.github.com>
Co-authored-by: Ana Paula Gomes <1899950+anapaulagomes@users.noreply.github.com>
@anapaulagomes
Copy link
Contributor

Pô, bem legal a funcionalidade de preview deploy do Netlify! Mas parece que não funciona atualmente. A mensagem que aparece nos logs é:

11:12:09 AM: Your next.config.js must set the "target" property to one of: serverless, experimental-serverless-trace.

Olhando umas issues por aí, parece que essa seria a solução: netlify/next-runtime#527 (comment) Pode ajudar nisso, @matheusrocha89? 🙏🏽

@matheusrocha89
Copy link
Collaborator

Posso, deixa comigo. Vou aproveitar e fazer commit das sugestões que dei.

@anapaulagomes
Copy link
Contributor

Massa, @filipebsmaia! :D Infelizmente o preview ainda não funciona. Mesmo com a porta configurada, ainda tem esse alerta:

Your next.config.js must set the "target" property to one of: serverless, experimental-serverless-trace. Update the
3:58:07 PM: target property to allow this plugin to run.

Pode ver isso?

@filipebsmaia
Copy link
Contributor Author

Massa, @filipebsmaia! :D Infelizmente o preview ainda não funciona. Mesmo com a porta configurada, ainda tem esse alerta:

Your next.config.js must set the "target" property to one of: serverless, experimental-serverless-trace. Update the
3:58:07 PM: target property to allow this plugin to run.

Pode ver isso?

Estou testando isso, se não der certo, vou reverter esses últimos commits.

@filipebsmaia
Copy link
Contributor Author

Funcionou @anapaulagomes 🎉

@anapaulagomes
Copy link
Contributor

Show! Obrigada, @filipebsmaia!

This was linked to issues Aug 19, 2021
Closed
@anapaulagomes anapaulagomes self-requested a review August 26, 2021 12:08
@anapaulagomes anapaulagomes merged commit 9549b17 into DadosAbertosDeFeira:main Aug 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configurar deploy no Netlify Blog
3 participants