Skip to content

Pantalla validacion#4

Open
brankitoCruz17 wants to merge 35 commits intodevelopmentfrom
pantalla-validacion
Open

Pantalla validacion#4
brankitoCruz17 wants to merge 35 commits intodevelopmentfrom
pantalla-validacion

Conversation

@brankitoCruz17
Copy link
Copy Markdown
Collaborator

Summary
Maquete pantalla validación.
Hice pantalla de validación, coloque imágenes, hice tableros y botones.
Tuve problemas para empezar, para usar flexbox y hacer los botones.

Screenshots
pantalla validacion

InVision
https://projects.invisionapp.com/d/#/console/19366632/404500035/preview

Trello
https://trello.com/c/ZYtnjSlb/16-maquetar-pantalla-validacion

brankitoCruz17 and others added 30 commits March 13, 2020 17:07
…ntre los cuadrados de los 24 - cambiar la tipografia del 24 - cambiar span por botones - bajar el cuadrado de resolucion un poco - centrar resolucion y botones
Copy link
Copy Markdown
Collaborator

@MMarussi MMarussi left a comment

Choose a reason for hiding this comment

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

Buen trabajo! Estamos cerca de tenerlo 👌

Hay 2 pares de componentes que tienen un compartamiento/estructura parecida (redButon/greenButton y ImagenSuperior/Inferior) en ambos casos se da que el jsx es muy parecido y el scss utilizado es casi el mismo excepto algun que otro atributo. Así no estaría mal fijarte que modificaciones tenés que hacer en algun par para eliminar sus duplicados y asi tener menos código que mantener!

function App() {
return <h1> Proyecto blockchain </h1>;
return (
<div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Me parece que este div sobra 👀

Comment on lines +9 to +71
<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>

<div className={styles.cell} >
<span className={styles.number}>24</span>
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Acá tenés repetido varias veces el mismo componente, acordate que en vez de tenerlo copiado y pegado varias veces podés hacerlo en menos líneas con una función.

function Board() {
  const renderCells = () =>
    [...Array(CELL_NUMBERS)].map(() => (
       <div className={styles.cell} >
          <span className={styles.number}>24</span>
        </div>)
    );

  return (
    <div className={styles.containerBoard}>
      <div className={styles.board}>
        {renderCells()}
      </div>
    </div>
  )
}

renderCells() va a crearnos un array vacío y por cada uno de esos elementos vamos a renderear una celda
CELLS_NUMBER es una constante con la cantidad de veces que se tiene que renderear una celda (creo que sería 16)

border-radius: 4px;
background-color: #FFFFFF;
margin-right: 21px;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Esta línea vacía se puede borrar

Comment on lines +32 to +43
.cell {
min-width: 25%;
min-height: 25%;
text-align: center;
display: flex;
justify-content: center;
align-items: center;
}

.cell:nth-child(odd) {
background-color: #FEC34D;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tené en cuenta que podés usar la sintaxis de SASS para anidar los pseudo selectores que apliquen a .cell

.cell {
  min-width: 25%;
  min-height: 25%;
  text-align: center;
  display: flex;
  justify-content: center;
  align-items: center;

  &:nth-child(odd) {
    background-color: #FEC34D;
  }

  &:nth-child(...) {
    background-color: ...;
  }
}

Comment on lines +45 to +129
.cell:nth-child(9) {
background-color: #F2754C;
}

.cell:nth-child(7) {
background-color: #F2754C;
}

.cell:nth-child(10) {
background-color: #22465C;
}



.cell:nth-child(16) {
background-color: #33BFC6;
}

.cell:nth-child(1) {
background-color: #AED750;
}

.cell:nth-child(2) {
background-color: #33BFC6;
}

.cell:nth-child(3) {
background-color: #AED750;
}

.cell:nth-child(4) {
background-color: #F2754C;
}

.cell:nth-child(5) {
background-color: #F2754C;
}

.cell:nth-child(6) {
background-color: #AED750;
}

.cell:nth-child(7) {
background-color: #22465C;
}

.cell:nth-child(8) {
background-color: #22465C;
}

.cell:nth-child(9) {
background-color: #FEC34D;
}

.cell:nth-child(10) {
background-color: #22465C;
}

.cell:nth-child(11) {
background-color: #F2754C;
}

.cell:nth-child(12) {
background-color: #22465C;
}

.cell:nth-child(13) {
background-color: #22465C;
}

.cell:nth-child(14) {
background-color: #FEC34D;
}

.cell:nth-child(15) {
background-color: #22465C;
}

.cell:nth-child(16) {
background-color: #FEC34D;
}

.cell{
border: 4px solid white;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

.cell:nth-child(odd) {
  background-color: #FEC34D;
}

El pseudo-selector anterior aplica el color #FEC34D al background. Es necesario este estilo si va a ser sobreescrito por todos estos?

Comment on lines +10 to +12
<span className={styles.iconContainer}>
<span className={styles.iconScore}>+</span>
</span>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Podrías combinar las clases de scss .iconContainer y .iconScore en una sola, ya que no tiene mucho sentido tener 1 <span> que sólo tiene otro <span> adentro

Comment on lines +1 to +6
.container{
display: flex;
justify-content: space-between;
margin-bottom: 36px;

}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.container{
display: flex;
justify-content: space-between;
margin-bottom: 36px;
}
.container {
display: flex;
justify-content: space-between;
margin-bottom: 36px;
}


function Text() {
return (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Esta línea se puede borrar

@@ -0,0 +1,13 @@
import React from 'react';

import Styles from './index.module.scss';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Intenta que los imports que se usen en el código no empiecen con mayúscula así no se los confunde con otro componente.

Suggested change
import Styles from './index.module.scss';
import styles from './index.module.scss';

@@ -0,0 +1,13 @@
import React from 'react';

import Styles from './index.module.scss';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mismo que Styles de redButton

Copy link
Copy Markdown
Collaborator

@MMarussi MMarussi left a comment

Choose a reason for hiding this comment

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

Sólo faltan unos detalles, ya casi estamoooos!!

Comment on lines +9 to +16
<Fragment>
<button type="button" className={styles.buttonRed} />
</Fragment>
</div>
<div className={styles.containerGreen}>
<Fragment>
<button type="button" className={styles.buttonGreen} />
</Fragment>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Estos 2 pares de fragments no son necesarios, así que podés borrarlos

border-radius: 4px;
transition: background-color .1s ease, transform .2s ease;
}
.containerGreen {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Acordate de separar clase y clase con una línea vacía

@@ -1,6 +1,6 @@
.lineasGrises {
.lineasGrises{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

El espacio separando nombre de clase y { estaba bien 😇

}

.button{
.buttons{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.buttons{
.buttons {

Comment on lines +28 to +30
.inferior {
transform: scaleY(-1);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ojo con la identación de este file (pasalo todo a 2 espacios):

Suggested change
.inferior {
transform: scaleY(-1);
}
.inferior {
transform: scaleY(-1);
}


function ImageLineas() {
return (
<div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Me parece que no necesitas este div

Comment on lines +1 to +4
.pantallaValidacion{
width: 1024px;
margin: 0 auto;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cuidado con la identación en todo el archivo (se están usando 4 espacios y en el proyecto usan 2):

Suggested change
.pantallaValidacion{
width: 1024px;
margin: 0 auto;
}
.pantallaValidacion {
width: 1024px;
margin: 0 auto;
}

html {
box-sizing: border-box;
}
*,*:before,*:after {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agrega una línea vacía para separar estos selectores

Comment on lines +7 to +25
.texto {
width: 384px;
color: #525256;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
}

.revision {
width: 92px;
color: #7758A2;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
margin-bottom: 36px;
margin-top: 14px;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cuidado con la identación:

Suggested change
.texto {
width: 384px;
color: #525256;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
}
.revision {
width: 92px;
color: #7758A2;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
margin-bottom: 36px;
margin-top: 14px;
}
.texto {
width: 384px;
color: #525256;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
}
.revision {
width: 92px;
color: #7758A2;
font-family: 'Montserrat', sans-serif;
font-size: 24px;
font-weight: bold;
letter-spacing: 0;
margin-bottom: 36px;
margin-top: 14px;
}

Comment on lines +146 to +149
.cell:nth-child(1n) {
border-bottom: 4px solid white;
border-left: 4px solid white;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fijate que nth-child(1n) en realidad terminaría aplicando a todos los hijos, asi que podés redefinir esto usando el selector hijo:
Suponiendo que cell es el padre de un div que usa la clase .block

.cell {
  > .block { }
}

Copy link
Copy Markdown
Collaborator

@MMarussi MMarussi left a comment

Choose a reason for hiding this comment

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

Bien ahí!!!

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