Push review for Vanessa to review #28
Conversation
…-be-implemented Chore/152345025/ui redesign to be implemented
- Error notification from controller handled by redux-notifications - Token return information modified to only take in a user id
- user Reducers implemented - Higher Order components introduced to protect routes - Tighter JSDocs introduced - Redirection of users on Login to Main Page - Validations for Sign In and Sign Up pages [finshes #152539728]
- Higher order components UserRoutes used to protect your routes.
- client side authentication now checks for invalid tokens redirecting users to the home page. - Logout feature fixed - Code refractored - styling added to scss - Code refactored
…for-sign-in-page Feature/152539728/ui redesign for sign in page
- Refactored test codes - Added a webpack.prod.config.js
- Group components in folders
- Token decoded at the server side - Welcome message for unauthenticated users implemented - User Book Routes refactored to reflect change in routes
- Borrowed books displayed - fetchbook by User id implemented
- Implemented pagination - Implemented Display for Userbooks
…for-sign-in-page UI redesign of main page
- Redirect to login page on successful signup - Implemented Auth Failure to assign state
- Message display if no books exist
- Eslint plugin re-installed to fix hound issues
- Images uploaded to cloudinary on onChange Modal function - Db retieved from cloudinary ready to be used in the onClick method - ProgressBar changed to use props - ImageReducer implemented and added to rootReducer
| @@ -2,13 +2,14 @@ import express from 'express'; | |||
| import controller from '../controllers'; | |||
| import fieldValidationMiddleware from '../controllers/middleware/fieldValidations'; | |||
| import nullvalidationMiddleware from '../controllers/middleware/nullValidation'; | |||
| import decodeToken from '../controllers/middleware/authenticate'; | |||
There was a problem hiding this comment.
Unable to resolve path to module '../controllers/middleware/authenticate' import/no-unresolved
Missing file extension for "../controllers/middleware/authenticate" import/extensions
client/src/app/css/style.scss
Outdated
|
|
||
| @media only screen and (min-width: 1600px) { | ||
| .overlay-main { |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| padding-left: 0; | ||
| } | ||
|
|
||
| .body-wrapper { |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| } | ||
|
|
||
| @media only screen and (max-width: 992px) { | ||
| header, |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| } | ||
|
|
||
| @media screen and (max-width: 541px) { | ||
| .signup-wrapper { |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| } | ||
|
|
||
| .main-text { | ||
| background-color: white; |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
Color white should be written in hexadecimal form as #ffffff
Color literals like white should only be used in variable declarations; they should be referred to via variable everywhere else.
There was a problem hiding this comment.
changed to the base white variable
|
|
||
| .react-tabs__tab--selected { | ||
| background: $base-orange; | ||
| color: $base-white; |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| } | ||
|
|
||
| .react-tabs__tab--selected { | ||
| background: $base-orange; |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| font-family: $font-stack; | ||
| } | ||
|
|
||
| .react-tabs__tab--selected { |
There was a problem hiding this comment.
Selector react-tabs__tab--selected should be written in lowercase with hyphens
|
|
||
| .react-tabs__tab-list { | ||
| padding-top: 15px; | ||
| font-family: $font-stack; |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
There was a problem hiding this comment.
Installed .sass-lint.yml to standardize my sass files
| @@ -0,0 +1,10 @@ | |||
| import userReducer from '../../src/app/reducers/userReducers'; | |||
|
|
|||
|
|
|||
| export const USER_LOGGED_OUT = 'USER_LOGGED_OUT'; | ||
|
|
||
| export const USER_SIGN_IN_FAILURE = 'USER_SIGN_IN_FAILURE'; | ||
| export const USER_SIGN_UP_FAILURE = 'USER_SIGN_UP_FAILURE'; | ||
|
|
||
| export const SIGNUP_USER_SUCCESS = 'SIGNUP_USER_SUCCESS'; |
There was a problem hiding this comment.
For consistency, try to make you variable names uniform. Here you have SIGNUP_USER_SUCCESS. With the way other variables are being named here, this should be USER_SIGNUP_SUCCESS
client/src/app/actions/api.js
Outdated
| /** | ||
| * | ||
| */ | ||
|
|
|
|
||
| import { USER_LOGGED_IN, | ||
| USER_LOGGED_OUT, | ||
| USER_SIGN_IN_FAILURE, | ||
| SIGNUP_USER_SUCCESS, |
There was a problem hiding this comment.
Also reflect the variable name change here.
| type: SET_CURRENT_USER, | ||
| user | ||
| }); | ||
|
|
| <Col l={4} s={4} m={4}> | ||
| Loading.... | ||
| <ProgressBar/> | ||
|
|
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import UserView from './UserView.jsx'; | ||
|
|
| <div className="col l3"> | ||
| <ReactTooltip /> | ||
| <div className="card" > | ||
| {/* <a |
| if(this.state.filename) | ||
| { | ||
| console.log('I am here', this.props.secure_url) | ||
| // imageUploadToDb |
There was a problem hiding this comment.
Zombie code. I will like you to replicate killing this wherever you have such existing.
| * @description Component for Message for No Books | ||
| * @class NoBooksMessage | ||
| */ | ||
| const NoBooksMessage = () => ( |
There was a problem hiding this comment.
I think this function could do with a better name.
- Updated the Readme.md file - Implemented a Display Modal for loaning a book - Implemented a pure component for the progress bar - Implemented an axios call for the loan book - Implement an Upload Image functionality
| @@ -0,0 +1,14 @@ | |||
| import React from 'react'; | |||
| import { NavLink } from 'react-router-dom'; | |||
There was a problem hiding this comment.
Unable to resolve path to module 'react-router-dom' import/no-unresolved
Missing file extension for "react-router-dom" import/extensions
| @@ -0,0 +1,14 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Unable to resolve path to module 'react' import/no-unresolved
Missing file extension for "react" import/extensions
|
|
||
| BookCoverUpload.propTypes = { | ||
| image: PropTypes.string, | ||
| username: PropTypes.string |
There was a problem hiding this comment.
'username' PropType is defined but prop is never used react/no-unused-prop-types
| } | ||
|
|
||
| BookCoverUpload.propTypes = { | ||
| image: PropTypes.string, |
There was a problem hiding this comment.
'image' PropType is defined but prop is never used react/no-unused-prop-types
| <a | ||
| onClick={this.onClick} | ||
| href="#!" | ||
| className="modal-action modal-close waves-effect waves-green btn-flat">Loan<i className="material-icons right">send</i> |
There was a problem hiding this comment.
Line 75 exceeds the maximum line length of 100 max-len
The closing bracket must be aligned with the line containing the opening tag (expected column 13 on the next line) react/jsx-closing-bracket-location
| </div> | ||
| <div className="modal-innercontent"> | ||
|
|
||
|
|
There was a problem hiding this comment.
Trailing spaces not allowed no-trailing-spaces
| <h4>Book</h4> | ||
| </div> | ||
| <div className="modal-innercontent"> | ||
|
|
There was a problem hiding this comment.
Trailing spaces not allowed no-trailing-spaces
More than 2 blank lines not allowed no-multiple-empty-lines
| * @memberof UploadPictureModal | ||
| * @return {void} | ||
| */ | ||
| render() { |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| description:'' | ||
| }; | ||
|
|
||
| } |
There was a problem hiding this comment.
Block must not be padded by blank lines padded-blocks
| categorty:'', | ||
| description:'' | ||
| }; | ||
|
|
There was a problem hiding this comment.
Trailing spaces not allowed no-trailing-spaces
- Added sass linting to the repository
| } | ||
|
|
||
| @media only screen and (min-width: 1600px) { | ||
| .overlay-main { |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
| } | ||
|
|
||
| .react-tabs__tab-list { | ||
| padding-top: 15px; |
There was a problem hiding this comment.
Line should be indented with spaces, not tabs
Properties should be ordered font-family, padding-top
benfluleck
deleted the
feature/152838221/implement-functionality-for-book-loans
branch
This PR has been issued against latest pull to my earliest pull on the repository
Description
This latest pull will show the amount of progress i have made in my Andela journey
What does this PR do?
This will help me become a world class developer
Description of Task to be completed?
Screenshots (if appropriate):
Any background context you want to provide?
What are the relevant pivotal tracker stories?
Types of changes
Checklist: