Skip to content

Conversation

@marco-souza
Copy link
Contributor

Descrição

Este PR adiciona a funcionalidade de alterar o tema da página.

Vale notar o uso do hook de cores useColorModeValue usado para definir uma cor para cada tema (light/dark).

Changes

  • Criei um botão para alternar entre light/dark themes (doc do chakra tem um video fazendo exatamente isso, recomendo)
  • Dei um tapa de estilos na navbar mobile
  • Adicionei o botão de alterar cor junto

Notes

  • @fmagesty - note que já estou usando os icones do chakra, acho que não precisamos importar outra lib de icones
  • @frattezi - removi o grid para usar o stack, acho que agora ficou confortável para um celular, da uma revisada please

This will avoid rerendering this list each time a state changes.
@marco-souza marco-souza requested review from Joel-leal and frattezi and removed request for frattezi and melipefelgaco August 17, 2021 15:46
@github-actions
Copy link
Contributor

github-actions bot commented Aug 17, 2021

Visit the preview URL for this PR (updated for commit 915975d):

https://podcodar-webapp--pr12-marco-add-dark-theme-km5zjbex.web.app

(expires Tue, 24 Aug 2021 15:55:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Contributor

@frattezi frattezi left a comment

Choose a reason for hiding this comment

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

Apenas algumas perguntinhas

const { isOpen, onOpen, onClose } = useDisclosure();
const navbarBgColor = useColorModeValue('gray.50', 'gray.900');
const actionButtons = useMemo(
() => [...socialIcons, <ToggleThemeButton key="toggle-theme" />],
Copy link
Contributor

Choose a reason for hiding this comment

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

não entendi mto bem o useMemo aqui com o array vazio, oque ele está otimizando neste caso? N seria o objetivo colocar a variável de colorMode como condicionador ou algo assim?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Na verdade o botão ja tem seus estados internos né. A ideia aqui é simplesmente evitar re-renderizar a actionButtons sempre que algum estado mudar.

Neste componentes temos por exemplo o estado de isOpen. Sempre que ele for atualizado, sem o useMemo a listagem será obrigatoriamente criada de novo, mesmo não precisando, já que suas props (inexistentes no caso) não mudaram.

Como o ToggleThemeButton não tem props, a lista de dependência do useMemo é vazia. Se ele tivesse alguma props, ai essa lista deveria conter todos os estado usados como dependências.

@marco-souza marco-souza merged commit 290e623 into main Aug 17, 2021
@marco-souza marco-souza deleted the marco/add-dark-theme branch August 17, 2021 21:48
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.

5 participants