-
Notifications
You must be signed in to change notification settings - Fork 686
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
Add purchase history component #392
Add purchase history component #392
Conversation
Generated by 🚫 dangerJS |
5e00a7c
to
6ed917b
Compare
} = this.props; | ||
|
||
const trimedTitle = | ||
title.length >= 19 ? `${title.substring(0, 19)}...` : title; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, why it is 19
? It better to avoid magical numbers. Secondly, it is better to create common util function trimString(str, maxLength)
or something like this and move the logic there. Thirdly, you can add ellipsis using css text-overflow: ellipsis;
// this.props.fetchPurchaseHistory(); | ||
// } | ||
|
||
// componentWillUnmount() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, do not add commented code to pull requests
id: 2, | ||
imageSrc: 'asd', | ||
title: 'consectetur adipiscing elit', | ||
date: '12,12,2018', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a strange format for date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's simply example of string formatted date, produced and put by the container component
display: flex; | ||
align-items: center; | ||
position: relative; | ||
padding: 17px 23px 9.5px 0px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a good practice to specify even number of pixels.
8682403
to
c8530bd
Compare
}; | ||
|
||
processDate(date) { | ||
return `${date.getDate()}, ${date.getMonth()}, ${date.getFullYear()}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move this to helpers.js file.
edefb67
to
3775fe7
Compare
3775fe7
to
17aea33
Compare
* Add purchase history component (#392) * Fix link and prop types (#407) * Finish item image * Simple test for static part of purchase history page (#417) * Add simple test for purchase history page and prettier a few files * Add tests for purchase history item component * Add some styles to filter component (#448) * Add semantic tags in list, prettier a few files (#443) * Convert px to rem * Address review comments * Update purchase history styles
This PR is a:
[ X ] New feature
[ ] Enhancement/Optimization
[ ] Refactor
[ ] Bugfix
[ ] Test for existing code
[ ] Documentation
Summary
When this pull request is merged, it will...
Additional information