-
Notifications
You must be signed in to change notification settings - Fork 310
Adjust viewport height for mobile devices on Update Shelter page #232
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
Adjust viewport height for mobile devices on Update Shelter page #232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coloca uns prints mostrando que resolveu e principalmente que não impactou outras resoluções.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Postei na issue original. Acredito que podemos resolver isso usando apenas css -> https://caniuse.com/viewport-unit-variants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analisando bem... Não precisaria nem mesmo o useEffect
com os listeners, apenas o cálculo do "vh custom" no topo do arquivo fora da "declaração do componente", pois na prática, o usuário não vai redimensionar a página no smartphone. Fiz alguns testes locais aqui e funcionaram normalmente (desktop e mobile), você pode alterar e fazer mais uns testes locais também, quanto menos a necessidade de useEffect
melhor., resolvido dessa forma mesmo com Tailwind:
document.documentElement.style.setProperty('--vh', `${window.innerHeight * 0.01}px`);
const UpdateShelter = () => {
// ...
return (
<div className="flex flex-col items-center h-[calc(var(--vh,1vh)*100)] md:h-screen">
// ...
Para telas maiores que 768 (md), pega o vh completo, abaixo disso, ele calcula.
Bem mais simples mesmo essa abordagem @allangalera, 92.34% dos browsers e versões já suportam essa unidade: @felipemrvieira, como temos um suporte para mais de 90%, considere o uso do dvh por gentileza. |
f994ed5
to
dc949f0
Compare
Acabei de atualizar o PR. Valeu pelo feedback! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Por essa abordagem tem uma nível de compatibilidade bem maior que o dvh
, uma vez que ele calcula a propriedade apenas no "primeiro load" da aplicação e usa a unidade px
, PR apto ao merge.
@SOS-RS/frontenders Preciso de mais um review aqui. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfeitooo
Acredito que está pronto para merge @vinny-silveira @andradeviniicius @larissapissurno |
mergeado @felipemrvieira |
ISSUE 225