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

Size Selector #79

Closed
wants to merge 13 commits into from
Closed

Size Selector #79

wants to merge 13 commits into from

Conversation

buuninbee
Copy link

@buuninbee buuninbee commented Feb 18, 2024

Closes #23

Feature

Criação do componente Seletor de Porte, esse componente tem três opções para o usuário descrever o tamanho do pet dele

Bugfix
  • Description
    N/A

  • Cause
    N/A

  • Solution
    N/A

Changelog N/A
Visual evidences 🖼️

Screenshot from 2024-04-05 11-45-38

document_4936182222253196287.mp4
Checklist
  • Issue linked
  • Build working correctly
  • Tests created
Additional info N/A

@buuninbee buuninbee changed the title slider do pet-dex Seletor de porte Mar 16, 2024
@buuninbee buuninbee marked this pull request as ready for review March 16, 2024 00:52
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.

O nome da pasta não está bom (está em português). A pasta de imagem não está no padrão das outras.

O componente está bem limitado e engessado, não consigo adicionar ou remover itens. Os nomes dos métodos não estão bons e não consigo reagir a alteração dos estados.

Não consigo obter o valor atual programaticamente.

Eu estou deixando o CSS com o @Alecell mas tem muita coisa errada, como os breakpoints não padronizados e a estilização por tag

src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/home/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
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.

Preciso que preencha corretamente o template da PR, aquela primeira etapa que ta com uns N/A, essas coisas tem que estar preenchidas de acordo!

Faltou criar o storybook do componente para documentação, da uma olhada nos arquivos .story que temos no projeto, pra visualiza-los usa pnpm storybook!

src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.js Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.scss Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.scss Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.scss Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.scss Outdated Show resolved Hide resolved
src/components/SeletorDePorte/index.scss Outdated Show resolved Hide resolved
@buuninbee buuninbee changed the title Seletor de porte Size Selector Mar 20, 2024
@buuninbee
Copy link
Author

O nome da pasta não está bom (está em português). A pasta de imagem não está no padrão das outras.

O componente está bem limitado e engessado, não consigo adicionar ou remover itens. Os nomes dos métodos não estão bons e não consigo reagir a alteração dos estados.

Não consigo obter o valor atual programaticamente.

Eu estou deixando o CSS com o @Alecell mas tem muita coisa errada, como os breakpoints não padronizados e a estilização por tag

O que exatamente não tá no padrão na pasta de imagem? o nome das img ?

@diogocaronte
Copy link
Contributor

O nome da pasta não está bom (está em português). A pasta de imagem não está no padrão das outras.
O componente está bem limitado e engessado, não consigo adicionar ou remover itens. Os nomes dos métodos não estão bons e não consigo reagir a alteração dos estados.
Não consigo obter o valor atual programaticamente.
Eu estou deixando o CSS com o @Alecell mas tem muita coisa errada, como os breakpoints não padronizados e a estilização por tag

O que exatamente não tá no padrão na pasta de imagem? o nome das img ?

Estamos padronizando como "images" e não "img"

src/home/index.html Outdated Show resolved Hide resolved
});
}

export default function SizeSelector({ slide }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export default function SizeSelector({ slide }) {
export default function SizeSelector({ slide } = {}) {

});
}

export default function SizeSelector({ slide }) {
Component.call(this, { html, events });

const slideItems = this.selected.get(slide);
Copy link
Contributor

Choose a reason for hiding this comment

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

Essa seleção pelo data-select está muito estranha pra mim, o data-select não foi pensando pra isso
@Alecell

}
},
},
{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{},

</div>
</body>
</html>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Não entendi por que esse arquivo foi alterado
E aqui está sem o EOF

Copy link
Member

@zoldyzdk zoldyzdk left a comment

Choose a reason for hiding this comment

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

E ae man, seria interessante um stories do componente ein, pra testar e etc

@Alecell
Copy link
Contributor

Alecell commented Jun 19, 2024

Task vai ser resolvida no PR #265

@Alecell Alecell closed this Jun 19, 2024
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 Componente Slider com Controle de Navegação
5 participants