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

Desafio 2 - Implementação do CSS3 #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

IT-maiara-costa
Copy link

No description provided.

@alinebezzoco
Copy link
Contributor

alinebezzoco commented Jan 6, 2019

Oi @IT-maiara-costa!
Bacana que você está usando o Flexbox para o posicionamento dos blocos e dos elementos!

A dica que eu te daria seria usar menos ids e mais classes. Id é recomendável usar 1x em cada página para referenciar algum bloco de elemento, por exemplo. Classes você pode repetí-las em diversos momentos.


<div id="container">

<div>
Copy link

@happymoon happymoon Jan 13, 2019

Choose a reason for hiding this comment

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

⚠️ Nessa parte aqui eu recomendo que você atribua uma classe pro <div> pra garantir que você pode a qualquer momento incluir um novo <div> alí dentro dos que já existem e não vai ter problema com herança do estilo.

Copy link
Author

@IT-maiara-costa IT-maiara-costa Jan 13, 2019

Choose a reason for hiding this comment

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

Darei uma mexida no código. As divs do id container, devem receberem as classes?

<p> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed </p>
</div>

<div>

Choose a reason for hiding this comment

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

⚠️

<p> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed </p>
</div>

<div>

Choose a reason for hiding this comment

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

⚠️

<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</p>

<hr />
Copy link

@happymoon happymoon Jan 13, 2019

Choose a reason for hiding this comment

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

🌶 O <hr /> tem uma função semântica de dividir conteúdos diferentes e nesse caso o parágrafo acima dele é totalmente relacionado ao conteúdo que está abaixo, então se é apenas uma questão visual recomendo colocar uma borda no topo ou abaixo do elemento mais próximo.

Copy link
Author

Choose a reason for hiding this comment

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

Entendi, obrigada pelo feedback,

@IT-maiara-costa
Copy link
Author

IT-maiara-costa commented Jan 13, 2019

@bezzocoaline Obrigada pelo feedback. Darei uma mexida no código.

Copy link

@happymoon happymoon left a comment

Choose a reason for hiding this comment

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

No geral você fez boas escolhas Maiara! Você está de parabéns.
A @bezzocoaline fez um comentário aqui também pertinente sobre a questão de usar classes ou ID's. O recomendável é que se use classes para tudo que possa ser reaproveitado, como cores, fontes e questões visuais. O ID deve ser único e utilizado para definir elementos dinâmicos (como campos de um formulário) ou seções importantes da página.

<p> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</p>

<hr />

Choose a reason for hiding this comment

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

🌶

<p> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</p>

<hr />

Choose a reason for hiding this comment

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

🌶


<h3>Mídias</h3>

<hr />

Choose a reason for hiding this comment

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

🌶

}


section > h1 {

Choose a reason for hiding this comment

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

💭 Não vejo motivo pra restringir o <h1> ao <section> aqui, isso deveria ser usado apenas para exceções e não para quando o caso se repete com tanta frequência. O ideal é que se utilize a hierarquia de título para definir a relevância do texto, então a princípio seus <h1> teriam o mesmo estilo, o que permite que você declare o h1 separado:

h1 {
	color:#EE1E53;
	text-align: center;
	padding-bottom: 2%;
	font-family: 'Source Sans Pro', sans-serif;
	font-size: 30px;
}

Copy link
Author

Choose a reason for hiding this comment

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

Eu limitei ao section pq estava alterando o comportamento do h1 da section welcome




#container div{

Choose a reason for hiding this comment

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

❗️ Nesses casos não recomendo usar o <div> como seletor, pois se daqui pra frente você quiser alterar esse elemento <div> por algum motivo semântico, você teria que reajustar todo seu CSS, então se estiver usando uma classe nesse <div> e trocá-lo para <span> não teria que ter todo esse trabalho pois seu CSS estaria referenciando uma classe e não o elemento.

}


#trabalhos div.works{

Choose a reason for hiding this comment

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

❗️ aqui há zero necessidade de especificar div.works, pode utilizar apenas .work

flex-wrap: wrap;
}

#trablahos div.works figure{

Choose a reason for hiding this comment

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

🖐 "trablahos" está digitado errado, então nada disso está sendo lido pelo browser.

Copy link
Author

Choose a reason for hiding this comment

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

Fui ver isso agora,, esqueci de remover essa parte

}


section#sobreeu h3:nth-of-type(1){

Choose a reason for hiding this comment

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

Essa gambiarra deve ser usada só quando extremamente necessário, dê preferência a atribuir uma classe pro H3 e referenciá-la no seu código


}

section#sobreeu h3:nth-of-type(2){

Choose a reason for hiding this comment

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

Nesse caso aqui vale atentar ao fato que os dois H3 que você usou dentro do #sobreeu tem a mesma aparência, então eles tem muita coisa em comum, portanto é bom declarar as coisas em comum apenas 1 vez e se necessário dar instruções específicas extras em outra linha.




#sobreeu h3 + p{ /* pega o p que esta abaixo do h3 */

Choose a reason for hiding this comment

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

Esse tipo de coisa é bom avaliar a necessidade de usar, pois qualquer alteração na estrutura do seu html pode fazer essa regra cair.

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.

3 participants