Aplicação: Weather Application

RicarGit commented 3 years ago

Fala Roger, passando pra deixar o link do repositório, inclusive no readme tem o link do netlify caso queira testar! --- https://github.com/RicarGit/Preject-Weather-Application ---

Não lembro se foi pedido pra ser feito uma nova versão, se foi, passou despercebido.

Falando em mudanças no visual, a única mudança que fiz foi colocar o estado correspondente à cidade logo ao lado dela!

Fiz a aplicação quase que sem ver os vídeos, fazia funcionar do meu jeito e depois via as aulas, ia mudando o código só conforme ia vendo como você fazia o seu, confesso que a maioria das mudanças eram nomes de constantes e funções.

Estou começando a pegar o jeito da refatoração, essa é minha maior dificuldade ainda mas já consigo refatorar alguns ifs pra ternário e curto circuito, destructuring é uma coisa que curto muito fazer mas ainda enrosco em perceber que posso fazer destructuring em alguns retornos de funções mas creio que tudo isso é questão de praticar mais pra ir entrando na cabeça!

Ler documentação ainda é uma dificuldade mas estou treinando isso também e olha que não tenho tanta dificuldade com o inglês.

Uma dúvida que me surgiu no momento que gostaria de tirar é que teve uma parte no meu código que eu fiz um ternário de um único if e não de um if else, se o if está alí só pra checar se algo vai retornar true pra aí eu fazer alguma coisa, eu posso refatorar pra um ternário e já que não da pra omitir o resultado logo após os dois pontos : poderia retornar false ou null, não sei se é uma boa prática isso...

Uma coisinha boba que aconteceu esses dias foi ter criado meu LinkedIn e ter feito o teste de competência de JavaScript e acabei recebendo o selo de competência em JavaScript e dizia que eu fiquei entre os 15% de 1M de pessoas que fizeram o teste, não sei se é grande coisa mas pelo menos dizia que o mínimo era estar entre 30% pra poder ganhar o selo, fora que o teste era todo em inglês... fiz o de HTML e passei entre os 30% e CSS reprovei, isso porque JavaScript era minha maior dificuldade, hoje tá sendo a tecnologia que eu mais sou bom haha

@Roger-Melo

Roger-Melo commented 3 years ago

Olá @RicarGit.

Passando para dizer que visualizei sua issue e se estiver tudo ok com a aplicação, em breve a análise e as respostas das dúvidas serão postadas aqui =)

Roger-Melo commented 3 years ago

Olá @RicarGit!

Parabéns pelo esforço em construir a terceira aplicação do treinamento =)

Vou deixar abaixo alguns pontos de atenção, segundo os critérios que usei para analisar.

Mas antes... fique atento aos critérios para enviar a aplicação. As cores da sua versão da aplicação devem ser diferentes das cores da aplicação mostrada na aula. Isso é fundamental para que a sua versão tenha um estilo próprio. Você pode usar o bootstrap ou outra lib CSS para evitar investir muita energia no design da interface.

Usabilidade

O problema em usar AdministrativeArea.ID é que ela não é muito precisa. Se vc pesquisar por "Tokyo", por exemplo, um número será renderizado e o usuário não entenderá o que aquele número quer dizer.

Organização de código

Boas práticas

Por convenção, invocações de funções costumam ficar na parte inferior do código. Então eu deixaria a invocação da showCityCard(cityCard) na última linha do bloco.

Evite usar ternários em casos onde eles não são necessários. Geralmente ternários não são usados para executar efeitos colaterais. O ideal é usá-los para fazer algo com o valor que eles retornam. Quando você precisar de um valor baseado em uma expressão, use ternário.

Nomes de variáveis e funções

img e regex são nomes vagos para consts. Você pode sintetizar no nome, de forma breve, do que se trata os valores que elas armazenam.

Refatoração

Na const isDayTime = IsDayTime, não faz sentido fazer esse tipo de atribuição para "trocar" o nome da const. Na verdade isso criou duas consts que armazenam booleans.

// ...
const isDayTime = IsDayTime
console.log(isDayTime, IsDayTime) // true true ou false false

O mesmo vale para const cityClimate = WeatherText.

Você pode fazer essa atribuição no destructuring:

const [{ IsDayTime: isDayTime, ... }]
// agora, apenas isDayTime está disponível para uso

Mas, não acho que trocar o nome dessas variáveis traz valor para o código. No fim das contas, acaba sendo um esforço que não compensa.

A insertWeatherInfoIntoDom é uma abstração válida para estudos, mas pode não valer à pena em uma aplicação em que você trabalha com mais pessoas (ou para o "você do futuro"). Uma desvantagem de abstrações é que elas tem um custo. Neste caso, a insertWeatherInfoIntoDom pode desacelerar o entendimento do código. Afinal, você no futuro ou uma outra pessoa, ao ler a invocação dela, terá que investigar o que ela faz. E o que ela faz é apenas uma atribuição de um texto como conteúdo de um elemento.

Ou seja:

insertWeatherInfoIntoDom(cityNameContainer, cityName)
insertWeatherInfoIntoDom(weatherClimate, cityClimate)
insertWeatherInfoIntoDom(weatherTemperature, cityCelsiusTemperature)

// descobrir o que as linhas acima fazem leva mais tempo do que descobrir o que o código abaixo faz

cityNameContainer.textContent = cityName
weatherClimate.textContent = cityClimate
weatherTemperature.textContent = cityCelsiusTemperature

textContent é mais familiar ao desenvolvedor enquanto insertWeatherInfoIntoDom é uma abstração a ser investigada.

Se quiser refletir mais sobre isso, dê uma olhada aqui:

E fica ligado, o valor no qual element.textContent = info resulta está sendo retornado pela função. Nesse caso não afeta o funcionamento dela, mas é importante lembrar disso.

Linhas em branco

Como você já sabe, linhas em branco podem ser usadas para indicar, visualmente, a separação entre conceitos.

Então eu removeria a linha 15 para agrupar as declarações das funções (já que elas só tem uma linha) e inseriria uma linha em branco na linha 39.

Uma coisinha boba que aconteceu esses dias foi ter criado meu LinkedIn...

Legal. Fico feliz em saber =)

As observações fizeram sentido?

RicarGit commented 3 years ago

As observações fizeram sentido?

Com certeza ajudaram muito, algumas coisas foram falta de atenção mesmo, como espaços, função no topo do código e variáveis que armazenei em outras variáveis! foram coisas que não deviam ter acontecido, pura falta de atenção hehe

Roger, eu dei uma refatorada no código, inclusive fiz com que dependendo do dia o background muda pra acompanhar a imagem e adicionei a cidade no localStorage, se você puder dar uma olhadinha rápida novamente eu agradeço!

Uma pergunta que surgiu na criação do código da localStorage, talvez mais pra frente você ensine mas só por curiosidade rápida mesmo, tem como armazenar toda informação que vem da API e então usar essa info pra não ficar fazendo request toda hora? eu sei que como a API da accuWeather precisa ser atualizada toda hora porque são informações em tempo real, mas deve ter casos que dá pra fazer isso certo? no caso guarda o objeto todo ou só as informações extraídas daí?

RicarGit commented 3 years ago

Roger, Aproveitando ainda essa aula e sobre seu feedback, logo após a aplicação da accuWeather teve o exercício de fazer o slide ser funcional, fiz esse código aqui e está funcionando perfeitamente mas só vou postar ele, gostaria que você desse uma olhada na estrutura que montei e refatorei já me baseando no seu feedback anterior.

Lembrando que eu não assisti a aula de correção ainda, verei assim que você der o seu feedback! Obrigado!

const carouselContainer = document.querySelector('.carousel')
const carouselImagesContainer = document
  .querySelectorAll('[data-js="carousel__item"]')

let slideIndex = 0
const lastImageIndex = carouselImagesContainer.length - 1

const hideImages = imagesContainer => {
  imagesContainer.forEach(img => {
    img.classList.remove('carousel__item--visible')
    img.classList.add('carousel__item--hidden')
  })
}

const checkClickedButton = clickedButton => {
  if (clickedButton === 'carousel__button--next') {
    slideIndex = (slideIndex >= lastImageIndex) ? 0 : ++slideIndex //coloquei entre ( ) pra melhor leitura
  }

  if (clickedButton === 'carousel__button--prev') {
    slideIndex = (slideIndex <= 0) ? lastImageIndex : --slideIndex //idem aqui
  }
}

const showCurrentSlideImage = slideIndex => {
  const currentSlide = carouselImagesContainer[slideIndex]

  currentSlide.classList.remove('carousel__item--hidden')
  currentSlide.classList.add('carousel__item--visible')
}

const manipulateSlider = event => {
  const clickedButton = event.target.dataset.js

  hideImages(carouselImagesContainer)
  checkClickedButton(clickedButton)
  showCurrentSlideImage(slideIndex)
}

carouselContainer.addEventListener('click', manipulateSlider)

Roger-Melo commented 3 years ago

Lembrando que eu não assisti a aula de correção ainda, verei assim que você der o seu feedback!

Não recomendo fazer isso. A aula com a correção do exercício está disponível. Eu tento responder as issues o mais rápido possível, mas as vezes acaba levando mais tempo do que eu gostaria.

Ficou legal a refatoração da Weather app. Apenas tome cuidado com o contraste entre a cor do texto superior ao input em relação à cor de background. Quando é noite na cidade, fica complicado ler.

Sobre o localStorage, na etapa 15 eu mostro uma técnica para evitar fazer muitos requests =)

O código do carousel é por aí mesmo. Mas pra eu testar com mais precisão aqui, fica melhor se vc subir todos os arquivos em um repositório.

@RicarGit

RicarGit commented 3 years ago

Não recomendo fazer isso. A aula com a correção do exercício está disponível. Eu tento responder as issues o mais rápido possível, mas as vezes acaba levando mais tempo do que eu gostaria.

Eu não ví pq eu sempre acabo pegando alguma coisa do vídeo e mudando, quis manter o código do meu jeito mesmo, sem "roubar" nomes de variáveis nem funções, não se preocupe, não pausei os estudos por conta disso.

O código do carousel é por aí mesmo. Mas pra eu testar com mais precisão aqui, fica melhor se vc subir todos os arquivos em um repositório.

Eu coloquei mais pra você visualizar a estrutura e os nomes mesmo, espero melhorar a organização do meu código!

Roger-Melo commented 3 years ago

Ah, saquei xD

Cara, no geral os nomes estão descritivos sim.

Eu apenas mudaria algumas coisas, tipo o nome da carouselImagesContainer. Já que ela contém um nodeList com vários itens, eu nomearia ela como images ou slides. Como vc nomeou a maioria com image, eu manteria o padrão.

lastImageIndex e slideIndex podem ser mais consistentes. Ou fica lastSlideIndex e slideIndex ou lastSlideImage e slideImage.

Como slideIndex representa o index do slide exibido atualmente, eu adicionaria "current" no começo do nome dela.

E eu tb evitaria ternários dentro de ifs (como mostrei na correção), porque no fim das contas é um aninhamento de condicionais e isso deixa o código mais complexo.

No mais, é isso =)

@RicarGit

RicarGit commented 3 years ago

Valeu pelas dicas Roger, tentarei manter padronizado os nomes também, rumo a fluência!

@Roger-Melo

Roger-Melo commented 3 years ago

Show! No que precisar, é só abrir uma nova issue =)

RicarGit commented 3 years ago

Sabe me dizer o porque do erro que dá no site do netlify? "TypeError: NetworkError when attempting to fetch resource" Quando vou na aba redes eu vejo que o status está com um circulo vermelho e na tranferência diz Mixed Block

Será que o netlify está bloqueando o fetch por achar não ser seguro?

@Roger-Melo

Roger-Melo commented 3 years ago

Olá @RicarGit. Manda os prints do erro, assim fica mais fácil ajudar =)

Ps: prefira não comentar em issues fechadas. Abra uma nova issue ou reabra a issue e comente. Se não, a issue não é exibida no painel de issues e eu posso não ver.

RicarGit commented 3 years ago

Ps: prefira não comentar em issues fechadas. Abra uma nova issue ou reabra a issue e comente. Se não, a issue não é exibida no painel de issues e eu posso não ver.

Vou lembrar disso na próxima haha, fica tranquilo Roger, ja resolvi, era o link que estava com http e não https, foi só acrescentar um s em http pro netlify parar de achar a conexão não segura.

Aproveitando aqui, pode me dar um spoiler do próximo modulo depois do firebase? vem TDD por aí? as vagas hoje pedem muuuito TDD como requisito obrigatório...

Desculpe atrapalhar @Roger-Melo, pode fechar a issue novamente, Obrigado!

Roger-Melo commented 3 years ago

Ah, show! =)

A etapa de TDD vai chegar, mas ainda não é a próxima. Você pode conferir o plano de aulas do treinamento aqui =)

@RicarGit

roger-melo-treinamentos / curso-de-js-roger-melo