search

KuoMinLi / ETF-DIY-F2E

ETF自由配 - 你就是王牌基金管理人
https://kuominli.github.io/ETF-DIY-F2E/#/
0 stars 0 forks source link

Code Review ETFItem.jsx #10

Open PoChenKuo opened 1 year ago

PoChenKuo commented 1 year ago

Please try to fix the variable name and refactor the component.

Your component name is ETFItem.jsx, but there is some layout relevant to the panel that exists. If you want to make a component named ETFItem, please ensure the context and layout are for the item only, which means you will render the item in the upper layer component.

data.map(itemData => <ETFItem {...itemData}) />)
PoChenKuo commented 1 year ago

https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L21 https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L24 etfName may be fit since you wrote the const { etfId } = useParams();

https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L51 https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L82 https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L115 https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L144 If you are using redux or similar stuff, try to push the promise into action.

https://github.com/KuoMinLi/ETF-DIY-F2E/blob/master/src/components/ETFItem.jsx#L10 I think this one isn't a component, thinking about the prop function