-
Notifications
You must be signed in to change notification settings - Fork 2
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
button component #4
base: master
Are you sure you want to change the base?
Conversation
X905
commented
Sep 22, 2020
- Sabor primary del boton
- Sabor secondary del boton
components/Button/Button.js
Outdated
export function Button(props){ | ||
const {className = '', ...restProps} = props; | ||
return( | ||
<button className={`bg-yellow text-black font-black text-xl px-6 py-5 hover:shadow-yellow ${className} `} {...restProps}> | ||
</button> | ||
|
||
) | ||
} | ||
|
||
|
||
export function ButtonGray(props){ | ||
const {className = '', ...restProps} = props; | ||
return( | ||
|
||
<Button className={`bg-grey hover:shadow-grey ${className}`} {...restProps}> | ||
</Button> | ||
) | ||
} | ||
|
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.
Hola @X905 , sugiero mantener una consistencia en el nombraniento de los componentes, por ejemplo el CamelCase en las funciones sirve para identificar que son un Componente, y para el ambito de React que podes usar hooks y lo demás de la libreria,
Por ello es sugerible que para el caso de estos componentes, cada componente cómo Button y ButtonGray estén en un archivo nombrados de la misma mánera
components/Button/Button.js
Outdated
export function ButtonGray(props){ | ||
const {className = '', ...restProps} = props; | ||
return( | ||
|
||
<Button className={`bg-grey hover:shadow-grey ${className}`} {...restProps}> | ||
</Button> | ||
) | ||
} |
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.
Semanticamente se entiende que este es un botón en gris, a diferencia de Button , que da entender que es solamente un botón, cuando la idea es tener 2 "sabores" del botón
components/Button/Button.js
Outdated
export function Button(props){ | ||
const {className = '', ...restProps} = props; | ||
return( | ||
<button className={`bg-yellow text-black font-black text-xl px-6 py-5 hover:shadow-yellow ${className} `} {...restProps}> |
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.
Es sugerible no hacer un spread de las props restProps
en el componente, en cambio sería mejor si las propiedades que este recibirá están detalladas con .propTypes, por ejemplo sabes que el botón tendrá un evento click, y en react la prop para los componentes para triggear el click es onClick