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

feat(dark-mode): adiciona suporte ao dark mode #377

Closed
wants to merge 18 commits into from
Closed

feat(dark-mode): adiciona suporte ao dark mode #377

wants to merge 18 commits into from

Conversation

ermesonsampaio
Copy link

Adiciona um alternador de temas

image

Dependência

Esse pull requeste depende da issue #375

Wrap BaseLayout with a Box that contains all content

#375
@vercel
Copy link

vercel bot commented May 24, 2022

Someone is attempting to deploy this pull request to the TabNews Team on Vercel.

To accomplish this, the commit author's email address needs to be associated with a GitHub account.

Learn more about how to change the commit author information.

pages/interface/components/DefaultLayout/index.js Outdated Show resolved Hide resolved
styles/bytemd.css Show resolved Hide resolved
const mode = localStorage.getItem('theme') || 'day';
const modeToChange = mode === 'day' ? 'night' : 'day';

localStorage.setItem('theme', modeToChange);
Copy link
Contributor

Choose a reason for hiding this comment

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

Pensando aqui. Será que não ficaria mais leagl salvar esses dados no perfil do usuário (no banco de dados) do que no localStorage?

Copy link
Author

Choose a reason for hiding this comment

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

Seria interessante, assim o tema iria ficar sincronizado em qualquer dispositivo que a pessoa logar com sua conta, porém acho pra implementar isso cabe abrir uma outra pull request

Copy link
Contributor

Choose a reason for hiding this comment

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

Boa! Será que vale abrir uma Issue?

Copy link
Author

@ermesonsampaio ermesonsampaio May 27, 2022

Choose a reason for hiding this comment

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

Acho super válido!

@vercel
Copy link

vercel bot commented May 27, 2022

@ermesonsampaio is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @ermesonsampaio needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@filipedeschamps
Copy link
Owner

Show @ermesonsampaio ! Estava testando localmente e vi que o Content perdeu (ou trocou) algumas estilizações no tema Light, olha que interessante:

Modo edit em Produção

image

Modo edit nessa Branch

image


Modo view em Produção

image

Modo view nessa Branch

image


E eu estou fascinado que você está em cima dessa issue e cada vez estamos nos aproximado. Mas de qualquer forma eu ainda sinto que talvez estamos usando alguma abstração errada (ou que ainda não está pronta) e por isso que está sendo necessário dar um balão "grande". Ponho entre aspas porque não é tão grande assim, mas eu realmente sinto que estamos precisando compensar por alguma coisa que não deveríamos estar compensando e esta compensação pode criar atritos na evolução e atualização dos componentes e dependências.

O que acha de listar o que barrou você de implementar nativamente o modo dark (por exemplo, o ByteMD), para talvez contatarmos os autores dos módulos a procura de mais informações, alternativas ou até ajustes do lado deles?

@ermesonsampaio
Copy link
Author

@filipedeschamps Essa mudança de estilos é no mínimo intrigante, é notável que no ambiente de Produção os estilos no modo View e Edit ficaram bem mais realçados, mas não sei até que ponto isso é realmente algo bom.

Como pedido vou descrever como foi o processo de implementação do dark mode.

Primer

Não houve nenhuma barreira por conta do hook useTheme.

ByteMD(View)

Houve uma certa dificuldade, pois anteriormente o tema vinha do github-markdown-css/github-markdown-light.css, porém a solução foi encontrada nessa issue primer/react#1680.

ByteMD(Edit)

Dentro do componente Editor do ByteMD existem 2 "áreas", sendo elas a área onde você escreve o markdown e a parte do layour do ByteMD.

A implementação do dark mode na parte onde você escreve o markdown foi simples, foi só adicionar uma propriedade editorConfig para o componente Editor e dentro dessa propriedade especificar o tema.

Na parte do layout do ByteMD foi necessário criar um arquivo css para que o layout ficasse em harmonia com o restante da aplicação.

Highlight.js

A solução encontrada foi utilizar sass (já que o sass já estava sendo usado na visualização do markdown), nesse arquivo basicamente é feito o load do css de acordo com o atributo data-theme da tag html.

@ermesonsampaio
Copy link
Author

ermesonsampaio commented May 29, 2022

Após fazer essa listagem meio que doucumentando as soluções encontradas, uma maneira de deixar o highlight da visualização do markdown no mesmo nível do ambiente de produção é usar a mesma solução encontrada para o Highlight.js, fazendo o load do css de github-markdown-css, assim não iriamos nem mais precisar do @primer/css.

A diferença que ainda continuaria seria só no modo Edit, por conta que no ambiente de produção o tema do Codemirror é o default, já nessa branch é o github-light (ou github-dark).

@ermesonsampaio
Copy link
Author

Agora o light mode nessa branch está assim como o tabnews em produção.

@filipedeschamps
Copy link
Owner

Showww!!! Dois pontos:

  1. Ta ficando realmente muito lindo!!! Topa fazer um rebase com a main para eu fazer um deploy no ambiente de Homologação pro pessoal ver?
  2. Toda vez que carrega a página está acontecendo um flash de conteúdo não estilizado. Isso acontece em ambos os temas inclusive. No início do vídeo abaixo eu faço um refresh no tema Light e ele pisca sem estilização do markdown, e entre as navegações quando o tema está escuro, fica mais evidente o efeito. Parece que a página primeiro carrega sem o tema, e depois escolhe entre o Light ou Dark. Possivelmente é algum comportamento assíncrono do React.
Screen.Recording.2022-05-30.at.8.53.54.AM.mov

@gabrnunes
Copy link
Contributor

Acho que isso é um problema recorrente do React mesmo, no meu projeto do Temcrase, eu usei uma lib para fazer o DarkMode e eles tem esse script aqui pra prevenir isso: https://github.com/gabrnunes/tem-crase/blob/main/public/noflash.js

Será que dá pra adaptar ele pro teu contexto @ermesonsampaio ?

@ermesonsampaio
Copy link
Author

@gabrnunes pelo que entendi basicamente esse script define uma classe no body de acordo com o tema no localStorage, então não sei se daria pra adaptar pra esse contexto por conta de que o tema é setado por meio do useTheme.

@filipedeschamps
Copy link
Owner

Publicado em Homologação 😍 e fiz um post de teste: https://tabnews-kyre21c3n-tabnews.vercel.app/filipedeschamps/dark-mode-no-tabnews-beta

Em paralelo, a Home fica sensacionalmente linda 🤝

Em paralelo 2, esse Dark Mode também faz passar mal o próprio ByteMD, olha o playground deles (na parte de syntax highlight): https://bytemd.js.org/playground/

image

Não ta fácil pra ninguém @ermesonsampaio 😂

@gpaiva00
Copy link
Contributor

Oi, pessoal! Não entendi onde paramos nessa questão do modo dark. Quem sabe eu possa ajudar em algo. 🤝🏽

@filipedeschamps
Copy link
Owner

Sofremos alguns detalhes técnicos na implementação, mas algo pode facilitar a nossa vida que é o GitHub estar criando um componente nativo dentro do Primer:

https://primer.style/react/drafts/MarkdownEditor
https://primer.style/react/drafts/MarkdownViewer

Não resolve todos os problemas, mas imagino que resolverá completamente o problema de estilização do editor.

@ermesonsampaio ermesonsampaio closed this by deleting the head repository Nov 21, 2022
@lucasalberto01
Copy link

@ermesonsampaio pode liberar seu repositório para eu poder ajudar ? 🥺

@ermesonqueiroz
Copy link
Contributor

@lucasalberto01 infelizmente perdi o acesso a essa conta.

@lucasalberto01
Copy link

@ermesonsampaio MEEEEEEEEEEEEE, vamos ter que escrever tudo de novo ? Pq agora não tem como atualizar essa PR com as correções solicitadas

@ErickCReis
Copy link
Contributor

Já que estão ressurgindo muitos pedidos a respeito dessa feature, vou resumir o maior impedimento, na minha visão, desde a última vez que dei uma olhada nisso.

Todas as principais páginas (home, recentes, username/slug, ...) são renderizadas no servidor (SSR), e nesse momento não é possível definir o tema do usuário, por conta disso a seleção do tema só é feita quando o navegador faz o hydration da página, o que causa esse problema #377 (comment).

Como o @aprendendofelipe disse aqui em cima, esse é um problema com o PrimerJs, que a princípio precisa de uma restruturação para aceitar variáveis css.

No meu último comentário eu implemento uma alternativa para tentar contornar esse problema, mas ela pode impactar no SEO e vai impedir o acesso de um usuário com o js desativado.

* Se eu tiver entendido errado ou já existir alguma outra solução por favor me avise

@lucasalberto01
Copy link

@ErickCReis eu acabei de fazer uma PR sobre isso (#916) e esse flash não está acontecendo na versão de dev nem na versão buildada.
Sabe dizer se esse comportamento só acontece online ?

@ErickCReis
Copy link
Contributor

@ErickCReis eu acabei de fazer uma PR sobre isso (#916) e esse flash não está acontecendo na versão de dev nem na versão buildada. Sabe dizer se esse comportamento só acontece online ?

Dei uma olhada lá, e eu acho que vc está caindo nesse caso aqui: #377 (comment)

Pode até parecer que está funcionando, mas o que está ocorrendo é que, por causa desse condicional if (!colorMode) return dentro de MyApp, não está sendo retornando mais nenhum HTML estático. Então esquece SEO, funcionar sem JS, etc.

@lucasalberto01
Copy link

@ErickCReis sabe alguma forma de testar e averiguá isso aqui em dev ?

@ErickCReis
Copy link
Contributor

@lucasalberto01 Não lembro como fiz da última vez, mas desativando o JS no dev tools vc deve ver esse erro, acho que só vai funcionar com a versão buildada.

@filipedeschamps
Copy link
Owner

Turma, caindo de paraquedas: minha sugestão é avaliar o componente oficial do Primer: https://primer.style/react/drafts/MarkdownEditor

Eu imagino que ele irá oferecer a compatibilidade "de graça" com dark mode 🤝

@aprendendofelipe
Copy link
Collaborator

Acho que ainda é como o @ErickCReis resumiu.

Turma, caindo de paraquedas: minha sugestão é avaliar o componente oficial do Primer: https://primer.style/react/drafts/MarkdownEditor

Eu imagino que ele irá oferecer a compatibilidade "de graça" com dark mode

Isso elimina só um dos problemas, que é o ByteMD, e é o mais fácil de resolver.

Ainda sobra o problema com a hidratação da página, que, até onde vi, para eliminar o flash e os problemas com SEO, só utilizando variáveis CSS, mas não é assim que o Primer faz a estilização.

Então temos que fazer uma "gambiarra" para estilizar os componentes do Primer com variáveis CSS. Nada impossível de fazer, mas daí ninguém garante que vai ser compatível com as atualizações do Primer.

@demetrius-mp
Copy link

demetrius-mp commented Nov 26, 2022

caindo de paraquedas aqui galera, mas ja tive problemas com fouc ao implementar dark mode (nao foi com next, foi com sveltekit)

na primeira implementação, eu salvava a variavel do tema no localStorage, dessa forma fica impossivel o servidor saber qual o tema do usuario pra ja enviar, por exemplo, uma classe "dark-mode" no <body> .

Consegui resolver o problema salvando o tema nos cookies, pq o servidor tem acesso aos cookies, entao bastou escrever algo como um middleware que pega o cookie do tema, e setar ele no body antes de enviar o html.

o problema no fim das contas se resume a salvar o tema do usuario em algum lugar que o servidor possa ver.

@manotroll
Copy link

não da pra liberar o recurso mesmo como beta ?
a tela piscar e ficar com o tema preto para mim esta bem assim ja que e uma forma de utilização ate resolver isso

@lucasalberto01
Copy link

lucasalberto01 commented Nov 28, 2022

@manotroll eu resolvi a tela piscando nessa PR #916.
Pelo oque eu vi ainda não subiu nenhuma atualização feita pela comunidade.

Não sei como está essa questão dos merges com o @filipedeschamps @aprendendofelipe

@aprendendofelipe aprendendofelipe marked this pull request as draft December 8, 2022 18:59
@danielschmitz
Copy link

danielschmitz commented Dec 16, 2022

EDIT: Dei uma olhadinha básica aqui, parece que a tag css dos elementos é dinamica e aleatória, sei lá pq. O que inviabiliza a minha ideia a seguir. Vi coisas como .eOokvn ao invés de link-visited. Enfim. ferrou KK

Pessoal, como o Dark Mode até agora deu alguns problemas na implementação, eu sugiro uma medida paliativa para ajudar os olhos dos nossos leitores vampiros.

1- Implementar, pelo menos, um Dark Mode apenas de leitura (nem todo mudo está editando coisas)
2- Não implementar a mudança/persistência de temas (menos é mais)
3- Observar o tema "Dark" do Sistema Operacional através do @media (prefers-color-scheme: dark) no css

basicamente, é colocar no arquivo css do tabnews algo como:

@media (prefers-color-scheme: dark) {

  /* Dark mode styles */
  .mat-drawer-container {
    background-color: #666;
  }

  .mat-card {
    background: #ddd;
  }

}

O exemplo acima é... apenas um exemplo da minha implementação aqui, de um projeto pessoal. Se concordarem posso até abrir um PR novo para a comunidade validar.

@aprendendofelipe
Copy link
Collaborator

@ermesonsampaio, acho que agora a conta correta é @ermesonqueiroz, né? Muito obrigado pelo PR!

E obrigado também pra toda a turma que participou por aqui, pois contribuíram muito para chegarmos na solução do PR #1348 que foi para produção hoje. 💪

O post comemorativo é esse: 🎉

https://www.tabnews.com.br/FelipeBarso/tabnews-dark-mode-e-novas-funcionalidades-do-editor

Vou fechar esse PR e qualquer ajuste necessário será mais fácil discutirmos em uma issue e resolver em um novo PR. 👍

Por exemplo a opção de salvar a preferência na conta do usuário, como sugerido pelo @gabrnunes, isso é algo que muito provavelmente será implementado futuramente 😉

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.