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

[MAIN][FEATURE] Button Component #108

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

AnaliaAlvarez
Copy link
Collaborator

Feature name

Button Component

Issue number

Crear componente Button con sus variantes #83

Tasks

  • Creo un componente Button con sus variantes "Light", "Light Outlined", "Light Disabled", "Light Outlined Disabled"
  • Agrego una página /demo/buttons con el único fin de visualizar el maquetado de los botones

Screenshot (1)

Copy link
Collaborator

@AnibalDBXD AnibalDBXD 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! El componente te quedo bastante bien, tiene algunos detalles a mejorar pero esta bastante bien 😄

modules/shared/components/buttons/button.tsx Outdated Show resolved Hide resolved
Comment on lines +1 to +7
import type { NextPage } from "next";

import Button from "@/modules/shared/components/buttons/button";
import { Container, Stack } from "@chakra-ui/react";

const ButtonsDemo: NextPage = () => {
const clickHandler = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Buena idea esta pagina!

Comment on lines 71 to 75
const sizeProps = {
sm: { padding: "10px 12px", gap: "8px", height: "40px" },
md: { padding: "10px 16px", gap: "8px", height: "44px" },
lg: { padding: "10px 24px", gap: "8px", height: "48px" },
}[size];
Copy link
Collaborator

Choose a reason for hiding this comment

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

este objeto podrias moverlo afuera del componente

y tendrias algo asi como:

const sizeProps = buttonSizes[size] ?? buttonSizes.md // <- esto es para tener un size por default si el size que le pasas no es correcto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moví los estilos de size y demas css al theme

Comment on lines 46 to 60
border: "1px solid #E2E8F0",
boxShadow: "0px 1px 3px rgba(0, 0, 0, 0.1), 0px 1px 2px rgba(0, 0, 0, 0.06)",
_hover: {
background: "#EDF2F7",
border: "1px solid #CFD3DC",
},
_focus: {
background: "#E2E8F0",
border: "1px solid #CFD3DC",
},
_disabled: {
opacity: 0.5,
border: "1px solid #CFD3DC",
},
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Esto tambien podrias moverlo afuera del componente

  if (outline) {
    props = {
      ...props,
      ...outlineStyles
    }
  }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

listo!

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