Skip to content

CODE REVIEW squad 4 Mariel #33

Open
marielcarrillo wants to merge 16 commits into
Laboratoria:masterfrom
marielcarrillo:master
Open

CODE REVIEW squad 4 Mariel #33
marielcarrillo wants to merge 16 commits into
Laboratoria:masterfrom
marielcarrillo:master

Conversation

@marielcarrillo

Copy link
Copy Markdown

Visualmente quedó bien, pero tengo unas dudas en cuanto el código de CSS y JS.
CSS:
el ancho de los botones a pesar de que los ponía del mismo tamaño aparecían de diferentes tamaños, lo mismo me paso con las cajas de texto donde una es un "text-area" y la otra un "p".
también a pesar de que los objetos quedaron en el lugar esperado, en el código lo hice con position: absolute y tuve que ir calando mediante "prueba y error" para atinarle a la posición deseada.
JS:
baje eslint y me marca que la función secondPage() esta definida pero no nunca usada, pero si me jala, no se a que se deba que salga ese error :/

Comment thread src/index.html
<div>
<button class="start-button" onclick="secondPage()"> COMENZAR </button>
<button class="start-button" onclick="secondPage()" title="changeBackground"> COMENZAR </button><!--secondPage() estoy llamando a la funcion desde js-->
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

La indentacion y los comentarios es algo que hay que cuidar.

Comment thread src/index.html Outdated
</section>
<section class="secondpage" id="secondPage"> <!--segunda pantalla-->
<section class="secondpage" id="secondPage"> <!--segunda pantalla -->
<div class="content-message">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Utiliza una conveción u otra, secondPage, second-page, pero usala en todo tu código

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok, ya lo cambie.

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