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

feat: Add RadioButton e Checkbox #90

Closed
wants to merge 7 commits into from
Closed

Conversation

jeressy
Copy link
Contributor

@jeressy jeressy commented Mar 1, 2024

Closes #25

Feature Adiciona os componentes RadioButton e Checkbox.
Visual evidences 🖼️
  • Checkbox:

Checkbox

  • Radio Button:

radio_btn

Additional info Ambos componentes foram documentados no storybook; Checkbox possui suporte para ter opções desativadas, conforme na imagem acima.

jeressy and others added 6 commits February 8, 2024 17:20
…ig.js

Adiciona os favicons necessários e configura o manifesto no plugin VitePWA no arquivo `vite.config.js`
Adiciona os componentes de radio button e checkbox, além de seus respectivos arquivos do storybook
Copy link
Contributor

@diogocaronte diogocaronte left a comment

Choose a reason for hiding this comment

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

Estou sentindo falta de um component para checkbox group e radio group.
@Alecell acha que isso foge do escopo da task?

src/components/Checkbox/index.js Outdated Show resolved Hide resolved
src/components/Checkbox/index.js Outdated Show resolved Hide resolved
src/components/Checkbox/index.js Outdated Show resolved Hide resolved
src/components/Checkbox/index.js Outdated Show resolved Hide resolved
src/components/Checkbox/index.js Outdated Show resolved Hide resolved
src/components/RadioButton/index.js Outdated Show resolved Hide resolved
src/components/RadioButton/index.js Outdated Show resolved Hide resolved
src/stories/Checkbox.stories.js Outdated Show resolved Hide resolved
src/stories/Checkbox.stories.js Show resolved Hide resolved
src/stories/RadioButton.stories.js Show resolved Hide resolved
src/components/Checkbox/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
Comment on lines +35 to +39
setChecked(isChecked = false) {
const $checkbox = this.selected.get('checkbox');
$checkbox.checked = isChecked;
this.emit("change", isChecked);
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Imagino que aqui precise de uma validação para evitar mudar o valor caso ele esteja desabilitado

Copy link
Contributor

@Alecell Alecell left a comment

Choose a reason for hiding this comment

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

Seu componente está muito bonito, de verdade! Aqui no storybook ta lindão, só precisamos fazer alguns ajustes!

Aparentemente seu PR ta usando 4 espaços de indentação, o projeto usa apenas 2, precisa fazer esse ajuste.

Outra coisa é que seus arquivos scss estão com alguns problemas nas unidades, por exemplo width, height, padding, margin e fontes. Todos devem usar outras unidades diferentes de px. Nesse artigo eu explico melhor essa questão das unidades

Os seus SCSS e JS tambem parecem que não estão lintados e nem seguindo o BEM, vc ta usando o nosso extension pack? Nele ja tem todas as extensões que vc vai precisar! PetdexExtensionPack

Sobre o BEM, vc pode ler sobre ele aqui: https://getbem.com/introduction/. Basicamente é o modo com estamos escrevendo nossas classes no projeto, uma forma tambem afim de impedir conflitos de classes!

Seu componente não deve ter uma margin padrão nele, essa margin será definida por quem precisar usar seu componente e não pelo seu componente em si! Assim damos liberdade a quem usa o componente de espaçar ele da fome que precisar
image

Seu PR veio com vários arquivos de imagem e uma alteração no main que não era pra fazer parte desse PR, precisamos ajustar isso!

São alterações simples a pesar de ser bastante! Na questão do rebase interativo, chame alguem pra te ajudar pra garantir que não vai perder nada!

No mais seu PR ta muito bom, a maior parte das observações são sobre padrão e não sobre coisas categoricamente erradas! Ta muito dahora, parabens!

src/components/Checkbox/index.scss Show resolved Hide resolved
src/components/Checkbox/index.scss Show resolved Hide resolved
src/components/Checkbox/index.scss Show resolved Hide resolved
src/components/Checkbox/index.scss Show resolved Hide resolved
src/components/Checkbox/index.scss Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.js Show resolved Hide resolved
src/components/RadioButton/index.scss Show resolved Hide resolved
src/home/index.html Show resolved Hide resolved
vite.config.js Show resolved Hide resolved
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.

Desenvolvimento de Componentes de Input Radio e Checkbox com Estados Dinâmicos
4 participants