feat(the-cart): add TheCart component #248
Conversation
There was a problem hiding this comment.
Boa noite @Raullages ,
Fiquei com a impressão que você só moveu os arquivos do componente praticamente, na verdade a refatoração passa por fixes no code style também e se conveniente uma revisão da API do componente (props, slots, events).
Além disto tem um ponto com um erro mais grave no trecho em que você importa um script interno do storefront-app, acho que o ideal na verdade seria torná-lo uma prop no TheCart.
Também notei umas quebras em identação e temos arquivos conflitantes agora porque recentemente eu também editei algumas coisas no mesmo componente, pode resolver os conflitos por favor?
Qualquer dúvida ou discordância só me dar um toque...
There was a problem hiding this comment.
@Raullages tenho mais algumas considerações sobre code style e convenções da API, mas o principal problema aqui é que faltou mover umas coisas que atualmente estão no EcCart, na sua refatoração perdemos umas features e fixes recentes.
There was a problem hiding this comment.
Melhorou bem @Raullages , mas comentei mais umas convenções e de novo (o mais importante) seu componente está desatualizado com o EcCart atual (no master), então perderíamos alguns feats/fixes na refatoração, o que não é legal 😅
Dá uma olhada lá de novo por favor, e nos comentários que eu deixei, valeu 👍
There was a problem hiding this comment.
Valeu @Raullages ,
Só comentei uns trecho que achei desnecessários e não entendi porque você colocou e tenho um problema com o nome que você escolheu canApplyCoupon porque na verdade o sentido é justamente o contrário, quando canApplyCoupon ("pode aplicar o desconto") está true é justando quando o cupon não é aplicado (porque já foi), então o nome atual tá indicando o contrário do que a variávei é na verdade, aí fica confuso 😕
Mas são só minors agora 👍
Related issue
#182