-
Notifications
You must be signed in to change notification settings - Fork 1
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
Creación de Componente de Carta (Cards) #13 #36
base: development
Are you sure you want to change the base?
Conversation
Adicionalmente, puedes proveer un |
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.
Podrias revisar este PR te ayudaria mucho ya que tiene las librerias instaladas. Podrias añadir algunos snapshot test para este componente. Aqui te dejo un ejemplo
src/components/developerCard.tsx
Outdated
@@ -0,0 +1,263 @@ | |||
import React from "react" | |||
import styled from "styled-components" | |||
import "@fortawesome/fontawesome-free/js/all.js" |
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.
@ppv95 esta libraria esta instalada en el repositorio?
import "@fortawesome/fontawesome-free/js/all.js" |
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.
@dionicio-gp La instale en el momento que estaban creado el componente. Lo elimine porque vi que ese mismo PR #34 lo tenia instalada y asumi que le haria merge antes que el mio. En dado caso, las pruebas si te las debo, las realizare antes de merge a development
src/components/developerCard.tsx
Outdated
<SocialNetworks> | ||
{webPage != null && ( | ||
<Media href={webPage} target="_blank"> | ||
<i className="fas fa-globe-americas"></i> |
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.
@ppv95 podrias usar la libreria de fontawasome para react
) | ||
|
||
it("renders card with initial data", () => { | ||
expect(card).toMatchSnapshot() |
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.
expect(card).toMatchSnapshot() | |
expect(card.toJSON()).toMatchSnapshot() |
Uno solo toma el contenido del component, no todo el objeto de testing.
developerName={initialData.DeveloperName} | ||
initials={initialData.Initials} | ||
image={initialData.Image} | ||
skillsList={initialData.SkillsList} | ||
summaryDeveloper={initialData.SummaryDeveloper} | ||
linkedin={initialData.linkedin} | ||
twitter={initialData.Twitter} |
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.
Puedes Reducir la cantidad de propiedades? creo que se puede resumir a solo un objeto llamado profile
y si queres, hasta podes implementar el spread operator:
<DeveloperCard {...profile} />
Esta es una sintaxis que si no estuviéramos usando TS, fuera indevido.
src/components/developerCard.tsx
Outdated
{webPage != null && ( | ||
<Media href={webPage} target="_blank"> | ||
<FontAwesomeIcon icon={faGlobeAmericas} /> | ||
</Media> | ||
)} | ||
|
||
{linkedin != null && ( | ||
<Media href={linkedin} target="_blank"> | ||
<FontAwesomeIcon icon={faLinkedin} /> | ||
</Media> | ||
)} | ||
|
||
{twitter != null && ( | ||
<Media href={twitter} target="_blank"> | ||
<FontAwesomeIcon icon={faTwitter} /> | ||
</Media> | ||
)} | ||
|
||
{github != null && ( | ||
<Media href={github} target="_blank"> | ||
<FontAwesomeIcon icon={faGithub} /> | ||
</Media> | ||
)} |
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.
Puedes colocarla, estas props puedes ponerle un valor null
de Default value, para evitar hacer la comparación. Si vas a comparar algo, es recomendable utilizar el triple igual ===
o la diferencia del triple !==
, porque compara tanto el valor del objeto como el prototipo
Subiendo componente carta para el issue #13.
Cualquier duda u oportunidad de mejora favor de hacermela saber.