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

Muda APIs do modo escuro #41

Merged
merged 1 commit into from
Mar 6, 2024
Merged

Conversation

gabrielguilhoto
Copy link
Contributor

@gabrielguilhoto gabrielguilhoto commented Feb 29, 2024

Contexto

Ao passar a usar as APIs do modo escuro no SGLearner, percebi que a implementação seria mais fácil se fossem feitas algumas mudanças no DS.

Mudanças

A mudança principal foi na função toggleTheme: antes ela simplesmente trocava o tema. Mas a interface que teremos para a escolha não será simplesmente um toggle mas um modal com as opções "claro" e "escuro". Daí achei que seria mais natural se o DS fornecesse uma função para mudar para um tema fornecido. Mudei a toggleTheme para uma função setTheme.

Houve também outras mudanças menores, que vou explicar com comentários no PR.

Como testar

  • Ver que testes automatizados continuam passando
  • Apontar o SGLearner para a branch do DS com as mudanças e ver que é possível usar a setTheme e outras APIs

useDarkMode,
defaultTheme,
} from './themes/DarkModeEnabledContext';
export * from './themes';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eu queria exportar outras coisas que estavam fora do arquivo DarkModeEnabledContext, então mudei para exportar tudo do themes como nos outros exports.

console.log(e);
});
return currentTheme;
const setAndStoreTheme = (theme: Theme): void => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essa é a mudança principal do PR: trocar o toggle por uma função que recebe o tema.

@@ -64,4 +64,4 @@ const DarkModeEnabledProvider: React.FC<{ children: ReactNode }> = ({
const useDarkMode = (): DarkModeEnabledContextProps =>
useContext(DarkModeEnabledContext);

export { DarkModeEnabledProvider, useDarkMode, defaultTheme };
export { DarkModeEnabledContext, DarkModeEnabledProvider, useDarkMode };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tirei o export do defaultTheme já que agora ele já é exportado pelo themes.ts. Adicionei o DarkModeEnabledContext para poder usar o .Consumer dele no SGLearner

@@ -61,25 +69,18 @@ test('DarkModeEnabledProvider theme is stored at toggle', async () => {

test('DarkModeEnabledProvider uses stored theme as current', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Esse teste passou a quebrar depois da mudança da toggleTheme e estava difícil de debugar e entender o que estava acontecendo.

Ele estava fazendo as queries com base no screen. Mudei para fazer elas com base no retorno do render, que é o padrão que a gente costuma usar no SGLearner, e com isso o problema deixou de acontecer. Também fiz alguns ajustes:

  • percebi que o expect dentro do setTimeout não estava sendo rodado na prática e troquei por um waitFor
  • removi o AsyncStorage.getItem e alguns thens que pareciam desnecessários

@@ -1,5 +1,7 @@
export type ThemeType = 'dark' | 'light';
export type Theme = 'dark' | 'light';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removi o "Type" do nome do tipo para seguir o padrão de tipos que usamos no SGLearner


export const defaultTheme: ThemeType = 'light';
export const labelsByTheme = { dark: 'Escuro', light: 'Claro' };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adicionei essa variável para usar como base para mostrar os nomes dos modos no SGLearner.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤌

@gabrielguilhoto gabrielguilhoto marked this pull request as ready for review March 5, 2024 12:40
ajuste no teste

Adicionando themes no files do package.json

exportando tb o context

adicionando exports

wip console logs

corrigindo teste e tirando console logs

removendo then desnecessário
@gabrielguilhoto gabrielguilhoto force-pushed the mudanca-api-modo-escuro branch from 36c5b46 to 4124f8a Compare March 6, 2024 13:09
Copy link
Contributor

@leocpadua7 leocpadua7 left a comment

Choose a reason for hiding this comment

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

Boa!!

@gabrielguilhoto gabrielguilhoto merged commit 7f4af01 into master Mar 6, 2024
1 check passed
@gabrielguilhoto gabrielguilhoto deleted the mudanca-api-modo-escuro branch March 6, 2024 17:26
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