Add price component #4
Conversation
react/Price.js
Outdated
|
|
||
| return ( | ||
| <div className='tc b fabriga'> | ||
| { listPriceElement } |
There was a problem hiding this comment.
geralmente não separamos que tá dentro do render em constantes. Não é legível. Coloca tudo no render e quando for necessário, faz um novo componente para ser reusado e ficar mais modularizado.
react/Price.js
Outdated
| * @property {number} [installmentPrice] - Single installment price | ||
| */ | ||
| static propTypes = { | ||
| sellingPrice: PropTypes.number.isRequired, |
There was a problem hiding this comment.
propTypes geralmente ficam depois da classe e não precisam ser estáticas 😬
Pode ser assim:
Price.propTypes = { sellingPrice: PropTypes.number.isRequired, ... }
react/Price.js
Outdated
| <FormattedMessage id='pricing.from'/> | ||
| </div> | ||
| <div className='dib strike ph2'> | ||
| { formatNumber(listPrice, currencyOptions) } |
There was a problem hiding this comment.
faltando indentação. Vcs usam lint e/ou prettier?
There was a problem hiding this comment.
Ainda não estamos utilizando nenhum. Qual deveríamos usar e como configurar?
There was a problem hiding this comment.
Tem um comando no CLI da VTEX para gerar uma configuração de lint vtex eslint local
There was a problem hiding this comment.
Adicionei os dois
react/Price.js
Outdated
| @@ -0,0 +1,96 @@ | |||
| import React, { Component } from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import { FormattedMessage, injectIntl } from 'react-intl'; | |||
There was a problem hiding this comment.
Importa também o intlShape para fazer a validação da prop intl.
intl: intlShape.isRequired
react/Price.js
Outdated
| <FormattedMessage id='pricing.from'/> | ||
| </div> | ||
| <div className='dib strike ph2'> | ||
| { formatNumber(listPrice, currencyOptions) } |
There was a problem hiding this comment.
Tem um comando no CLI da VTEX para gerar uma configuração de lint vtex eslint local
pages/pages.json
Outdated
| "component": "Price", | ||
| "props": { | ||
| "showListPrice": true, | ||
| "listPrice": 120, |
There was a problem hiding this comment.
@brunojdo comentei com @estacioneto sobre esse código hard-coded. A gente costuma não aceitar nada assim aqui, mas também sei que o projeto está iniciando agora. O que acha? Sugeri deixar on hold enquanto a lógica não é implementada
There was a problem hiding this comment.
Além disso, este trecho é apenas para mostrar o componente funcionando como extension point na tela principal. Posso remover se preferir, @brunojdo . Removo esta parte e mostro pessoalmente o funcionamento
There was a problem hiding this comment.
Concordo com @analuizamtg tem que tirar. Quando chegar eu vejo o funcionamento.
react/index.js
Outdated
| @@ -3,6 +3,8 @@ import PropTypes from 'prop-types' | |||
| import {ExtensionPoint, Link, Helmet} from 'render' | |||
| import {FormattedMessage, FormattedHTMLMessage, injectIntl, intlShape} from 'react-intl' | |||
There was a problem hiding this comment.
Esse componente FormattedHTMLMessage está sendo usado em algum lugar?
There was a problem hiding this comment.
Não achei também. Dá uma checada @estacioneto
There was a problem hiding this comment.
Não. Removi. Apesar de que foi importado automaticamente pelo boilerplate do projeto.
pages/pages.json
Outdated
| "component": "Price", | ||
| "props": { | ||
| "showListPrice": true, | ||
| "listPrice": 120, |
There was a problem hiding this comment.
Concordo com @analuizamtg tem que tirar. Quando chegar eu vejo o funcionamento.
react/index.js
Outdated
| @@ -3,6 +3,8 @@ import PropTypes from 'prop-types' | |||
| import {ExtensionPoint, Link, Helmet} from 'render' | |||
| import {FormattedMessage, FormattedHTMLMessage, injectIntl, intlShape} from 'react-intl' | |||
There was a problem hiding this comment.
Não achei também. Dá uma checada @estacioneto
react/Price.js
Outdated
| intl: intlShape.isRequired, | ||
| } | ||
|
|
||
| Price.schema = { |
There was a problem hiding this comment.
Quem tem schema é o Product Summary. Retirar
No description provided.