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

Create a Size Selector Component #265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/components/SizeSelector/images/large.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/components/SizeSelector/images/medium.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/components/SizeSelector/images/small.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

164 changes: 164 additions & 0 deletions src/components/SizeSelector/index.js
JonasGz marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
import { Component } from 'pet-dex-utilities';
import small from './images/small';
import medium from './images/medium';
import large from './images/large';

import './index.scss';

const events = ['event'];

const html = `
<div class="container-size-selector" role="radiogroup">
<ul class="container-size-selector__size-list" data-select="sizelist">
<li class="container-size-selector__card" role="radio" aria-checked="false" tabindex="0">
<div class="container-size-selector__card-size">
<div class="container-size-selector__container-img">
${small}
</div>
<div class="container-size-selector__container-text">
<h3 class="container-size-selector__title">Small</h3>
<span class="container-size-selector__text">under 14kg</span>
</div>
</div>
</li>
<li class="container-size-selector__card container-size-selector__card--active" data-select="card-active" role="radio" aria-checked="true" tabindex="0">
<div class="container-size-selector__card-size">
<div class="container-size-selector__container-img">
${medium}
</div>
<div class="container-size-selector__container-text">
<h3 class="container-size-selector__title">Medium</h3>
<span class="container-size-selector__text">14-25kg</span>
</div>
</div>
</li>
<li class="container-size-selector__card" role="radio" aria-checked="false" tabindex="0">
<div class="container-size-selector__card-size">
<div class="container-size-selector__container-img">
${large}
</div>
<div class="container-size-selector__container-text">
<h3 class="container-size-selector__title">Large</h3>
<span class="container-size-selector__text">over 25kg</span>
</div>
</div>
</li>
</ul>
</div>
`;

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

this.$sizeList = this.selected.get('sizelist');
this.$cards = Array.from(
this.$sizeList.querySelectorAll('.container-size-selector__card'),
);

const addEventListeners = (card, index) => {
card.addEventListener('click', () => {
this.selectCard(card, index);
this.getCardActiveEvent('click', card, index);
});

card.addEventListener('keydown', (key) => {
this.handleKeyDown(key, card);
});
};

this.$cards.forEach((card, index) => addEventListeners(card, index));
}

SizeSelector.prototype = Object.assign(
SizeSelector.prototype,
Component.prototype,
{
setText(index, title, subtitle) {
const h3 = this.$cards[index].querySelector('h3');
const span = this.$cards[index].querySelector('span');
h3.textContent = title;
span.textContent = subtitle;
},
JonasGz marked this conversation as resolved.
Show resolved Hide resolved

handleKeyDown(event, card) {
let next;
let prev;
let nextIndex;
let prevIndex;
if (event.key === 'ArrowRight' || event.key === 'ArrowDown') {
next = card.nextElementSibling;
if (next) {
nextIndex = this.$cards.indexOf(next);
next.focus();
this.selectCard(next, nextIndex);
this.getCardActiveEvent('keydown', next, nextIndex);
}
}
if (event.key === 'ArrowLeft' || event.key === 'ArrowUp') {
prev = card.previousElementSibling;
if (prev) {
prevIndex = this.$cards.indexOf(prev);
prev.focus();
this.selectCard(prev, prevIndex);
this.getCardActiveEvent('keydown', prev, prevIndex);
}
}
Comment on lines +90 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

Seria legal separar isso em funções, acredito inclusive que vc consegue fazer apenas 1 função que lida com o "proximo" item, passando os parametros corretos ele sabe lidar se é um item seguinte ou um item anterior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

consegui reduzir pra uma função apenas mas nao sei se daria p melhorar, vou conversar ctg

},

selectCard(selected, index) {
this.addActive(selected, 'container-size-selector__card--active');
this.centerCard(selected, index);
},

centerCard(card, index) {
if (index === 0)
this.$sizeList.classList.add(
'container-size-selector__size-list--active-padding',
);
Comment on lines +116 to +119
Copy link
Contributor

Choose a reason for hiding this comment

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

Oq isso faz?

Pq precisamos disso?

Outra coisa, quando vc tem um conteudo de um if mais complexo sempre abra o bloco do if, isso q vc fez funciona, mas prejudiuca muito a leitura

Copy link
Contributor Author

@JonasGz JonasGz Jul 9, 2024

Choose a reason for hiding this comment

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

o motivo disso é pelo seguinte:
eu preciso adicionar um padding nas laterais para ter espaço para o primeiro card e o último scrollarem para o centro.
no entanto, quando eu adiciono o padding da lateral esquerda na inicialização, os cards já iniciam totalmente para a direita colocando o primeiro card ao centro, que é diferente do comportamento que esperamos de ter o card do meio no centro e não o primeiro.
Para resolver isso eu coloquei para o componente iniciar sem padding a esquerda e adicionar apenas quando eu quiser que o primeiro card vá para o centro que é clicando no mesmo.

adicionei o bloco nos if


const cardRect = card.getBoundingClientRect();
const listRect = this.$sizeList.getBoundingClientRect();
const cardCenter = cardRect.left + cardRect.width / 2;
const listCenter = listRect.left + listRect.width / 2;
const offset = cardCenter - listCenter;
Comment on lines +121 to +125
Copy link
Contributor

Choose a reason for hiding this comment

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

To com a sensaçào de que esse não é o melhor modo de fazer...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vou conversar ctg


this.$sizeList.scrollBy({
left: offset,
behavior: 'smooth',
});
},

addActive(element, className) {
JonasGz marked this conversation as resolved.
Show resolved Hide resolved
this.$cards.forEach((card) => {
card.setAttribute('aria-checked', 'false');
card.classList.remove(className);
});
element.classList.add(className);
element.setAttribute('aria-checked', 'true');
},

getCardActiveEvent(eventName, card, index) {
Copy link
Contributor

Choose a reason for hiding this comment

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

O nome dessa função ta meio ruim, deveria ser trigger events ou algo assim, get usamos quando pegamos algo, nesse caso não estamos pegando nada hm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

poderia ser emitCardEvent?

if (eventName === 'click') {
this.emit('event', card, index);
}
if (eventName === 'keydown') {
this.emit('event', card, index);
}
},

getCardActive() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Esse nome tambem não me parece correto. No caso vc ta retornando, mas seria getActiveCard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrigi para getActiveCard e to pensando se da pra usar outro nome

const $activeCard = this.$cards.find((element) =>
element.classList.contains('container-size-selector__card--active'),
);
const indexCard = this.$cards.findIndex((element) =>
element.classList.contains('container-size-selector__card--active'),
);
return {
card: $activeCard,
index: indexCard,
};
},
},
);
190 changes: 190 additions & 0 deletions src/components/SizeSelector/index.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
@use '~styles/colors.scss' as colors;
@use '~styles/breakpoints.scss' as breakpoints;
@use '~styles/fonts.scss' as fonts;

.container-size-selector {
width: 100%;
max-width: 60rem;

display: flex;

&__size-list {
width: 100%;
overflow-x: hidden;

display: flex;
flex-direction: row;
gap: 1rem;

padding: 5rem 40% 5rem 0;

transition: padding 0.4s;

&--active-padding {
padding: 5rem 40%;
}
}

&__card:focus {
outline: none;
}

&__card {
JonasGz marked this conversation as resolved.
Show resolved Hide resolved
display: flex;

align-items: center;
justify-content: center;

margin: 0 0.7rem;

padding: 1rem 0;

box-sizing: border-box;

background-color: colors.$secondary100;
box-shadow: 0 0 0.3rem 0.3rem colors.$gray150;
border-radius: 1.8rem;

transition: transform 0.3s;

&:focus {
outline: 0.3rem solid colors.$primary200;
}

&:hover {
outline: 0.3rem solid colors.$primary200;

.container-size-selector__svg path {
fill: colors.$primary200;
}

.container-size-selector__title {
color: colors.$primary200;
}

.container-size-selector__text {
color: colors.$primary200;
}

.container-size-selector__container-img {
background-color: rgb(209, 230, 255);
}
}

&--active {
transform: scale(1.3);
outline: 0.3rem solid colors.$primary200;

transition: transform 0.3s;

.container-size-selector__svg path {
fill: colors.$primary200;
}

.container-size-selector__title {
color: colors.$primary200;
font-size: fonts.$xs;
}

.container-size-selector__text {
color: colors.$primary200;
font-size: fonts.$xxs;
}

.container-size-selector__container-img {
width: 4.5rem;
height: 4.5rem;

background-color: rgb(209, 230, 255);
}
}
}

&__card-size {
width: 9.4rem;
height: 10.6rem;
JonasGz marked this conversation as resolved.
Show resolved Hide resolved

display: flex;
flex-direction: column;
gap: 1.5rem;

align-items: center;
justify-content: center;
}

&__container-img {
width: 4rem;
height: 4rem;
JonasGz marked this conversation as resolved.
Show resolved Hide resolved

display: flex;

align-items: center;

justify-content: center;

background-color: colors.$gray150;
border-radius: 100%;
}

&__container-text {
display: flex;
flex-direction: column;
gap: 0.3em;

align-items: center;

justify-content: center;
}

&__title {
font-family: fonts.$primaryFont;
color: colors.$gray800;
font-size: fonts.$xs;
font-weight: fonts.$semiBold;
}

&__text {
font-family: fonts.$primaryFont;
color: colors.$gray600;
font-size: fonts.$xxs;
font-weight: fonts.$regular;
}
}

@include breakpoints.from360 {
.container-size-selector {
&__size-list {
gap: 1.5rem;
}

&__card {
padding: 1rem 0;
}
}
}

@include breakpoints.from667 {
.container-size-selector {
&__size-list {
gap: 5rem;
}

&__card {
padding: 4rem 2rem;

&--active {
.container-size-selector__title {
font-size: fonts.$sm;
}

.container-size-selector__text {
font-size: fonts.$xs;
}
}
}

&__text {
font-size: fonts.$xs;
}
}
}
Loading