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

chore(header): option brand in menu / header #916

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

matheusgnreis
Copy link
Member

Related issue

Support demand

Screenshots

aside menu
Screenshot 2023-06-14 at 17-08-25 My Shop
Screenshot 2023-06-14 at 17-08-41 My Shop

header

Captura de tela 2023-06-14 170745

Checklist

  • I've read and understood contributing guidelines;
  • If changing UI, I've tested on most common viewports, desktop and mobile devices;

@matheusgnreis
Copy link
Member Author

Quiser deixar pra outra versão, não tem problema..., pq ja tem algumas demandas ai pra entrar

const { data } = await _.ecomClient.store({
url: '/brands'
})
categoryParents['_'].push({
Copy link
Member

Choose a reason for hiding this comment

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

Talvez esse categoryParents deveria ser renomeado pra algo que não remetesse só a categoria então

let brands
if (hasBrands) {
try {
const { data } = await _.ecomClient.store({
Copy link
Member

Choose a reason for hiding this comment

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

Problema é que você duplica a request pra buscar essas marcas né?

Copy link
Member

Choose a reason for hiding this comment

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

Deixa em um menu só (mais fácil) ou crie uma abstração que evite que o usuário faça essa configuração com a request duplicada

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.

None yet

2 participants