4
\$\begingroup\$

I'm new to React. I major in Angular. And for me it's realy not so obvious here... Do I use this props & state variables in a good way or do I need to refactor/change something?

Main goal is such: simple app with table, search & pagination. URL-state with query params should lead to a proper page, sorting etc when somebody copy-paste the url. All params in URL are optional (so I do not like the idea of using url like /:offset/:limit/:search etc).

My repository: https://bitbucket.org/kot_matpockuh/react_test_app/src/

Maybe I have any other weird code?

tractorsList.js

import React, {Component} from 'react';
import TractorService from '../../services/tractor';
import PaginationBar from '../../shared/pagination/pagination';
import TableHeader from '../../shared/tableHeader/tableHeader';
import {GlobalConst} from '../../constants/global';
import * as qs from 'query-string';
class TractorList extends Component {
 // define state variables
 state = {
 currentPage: 0,
 pageParams: {
 offset: 0,
 limit: GlobalConst.defaultPageLim,
 sortField: undefined,
 sortOrder: undefined,
 search: undefined
 },
 tractors: {
 total: 0,
 data: []
 }
 };
 //define class variables
 isLoading = false;
 componentWillMount() {
 //parse URL-params (when somebody puts a link with params into browser)
 const parsedUrl = qs.parse(window.location.search);
 const objKeys = Object.keys(parsedUrl);
 const tempStateData = this.state.pageParams;
 for (const keyItem of objKeys) {
 if (this.state.pageParams.hasOwnProperty(keyItem)) {
 if (keyItem === 'offset' || keyItem === 'limit') {
 tempStateData[keyItem] = Number(parsedUrl[keyItem] || 0);
 } else {
 tempStateData[keyItem] = parsedUrl[keyItem];
 }
 }
 }
 this.setState({
 pageParams: tempStateData
 }, () => {
 // to be sure: state have updated already & update current pagination page
 if (this.state.pageParams.offset > 0) {
 this.setState({
 currentPage: Math.ceil(this.state.pageParams.offset / this.state.pageParams.limit)
 });
 }
 });
 }
 componentDidMount() {
 // after all URL & state manipulations: get data from the server
 this.getTractors();
 }
 getTractors() {
 // here we use service
 this.clearTractorsList();
 this.isLoading = true;
 const data = this.state.pageParams;
 TractorService.getTractors(data)
 .finally(() => {
 this.isLoading = false;
 })
 .then(res => {
 this.setState({tractors: res.data});
 })
 }
 clearTractorsList() {
 // first of all clear existing list
 this.setState({
 tractors: {
 total: 0,
 data: []
 }
 });
 }
 changePage(pageNr, limit) {
 // get data from pagination component and transform it & change URL
 const offset = (pageNr) * limit > 0 ? (pageNr * limit) : 0;
 const statePageParams = this.state.pageParams;
 statePageParams.offset = offset;
 statePageParams.limit = limit;
 this.setState({
 currentPage: pageNr,
 pageParams: statePageParams
 });
 this.props.history.push({
 pathname: '/tractors',
 search: qs.stringify(this.state.pageParams)
 });
 this.getTractors();
 }
 setSorting(sortField, sortOrder) {
 // get data from table-header component and transform it & change URL
 const statePageParams = this.state.pageParams;
 statePageParams.offset = 0;
 statePageParams.sortField = sortField;
 statePageParams.sortOrder = sortOrder;
 this.setState({
 currentPage: 0,
 pageParams: statePageParams
 });
 this.props.history.push({
 pathname: '/tractors',
 search: qs.stringify(this.state.pageParams)
 });
 this.getTractors();
 }
 setSearchValue(val){
 // get data from search input, transform it
 const statePageParams = this.state.pageParams;
 statePageParams.offset = 0;
 statePageParams.search = val;
 this.setState({
 currentPage: 0,
 pageParams: statePageParams
 });
 }
 searchTractors(event) {
 // change URL & get data
 event.preventDefault();
 this.props.history.push({
 pathname: '/tractors',
 search: qs.stringify(this.state.pageParams)
 });
 this.getTractors();
 }
 checkIfFieldIsSorted(field) {
 // check: was field sorted or not?
 return this.state.pageParams.sortField === field;
 }
 checkWayOfSorting() {
 // check: was field sorted asc/desc?
 return this.state.pageParams.sortOrder === 'asc' ? true : false;
 }
 render() {
 return (
 <div>
 <form onSubmit={event => this.searchTractors(event)}>
 <div className="input-group mb-3">
 <input type="text" className="form-control" placeholder="Find..."
 onChange={e => this.setSearchValue(e.target.value)}/>
 <div className="input-group-append">
 <button className="btn btn-outline-secondary" type="submit">⌕</button>
 </div>
 </div>
 </form>
 <div className="grid-table">
 <div className="grid-table-header">
 <div className="grid-table-header-col">
 <TableHeader text={"Reg. Number"} field={"registrationNumber"}
 isActive={this.checkIfFieldIsSorted("registrationNumber")}
 sortOrder={this.checkWayOfSorting()}
 onSortingChanged={this.setSorting.bind(this)}></TableHeader>
 </div>
 <div className="grid-table-header-col">
 <TableHeader text={"Manufacturer"} field={"manufacturer"}
 isActive={this.checkIfFieldIsSorted("manufacturer")}
 sortOrder={this.checkWayOfSorting()}
 onSortingChanged={this.setSorting.bind(this)}></TableHeader>
 </div>
 <div className="grid-table-header-col">
 <TableHeader text={"Model"} field={"model"}
 isActive={this.checkIfFieldIsSorted("model")}
 sortOrder={this.checkWayOfSorting()}
 onSortingChanged={this.setSorting.bind(this)}></TableHeader>
 </div>
 <div className="grid-table-header-col">
 <TableHeader text={"Actual Mileage"} field={"actualMileage"}
 isActive={this.checkIfFieldIsSorted("actualMileage")}
 sortOrder={this.checkWayOfSorting()}
 onSortingChanged={this.setSorting.bind(this)}></TableHeader>
 </div>
 </div>
 <div className="grid-table-body">
 {
 this.state.tractors.data.map(tractor =>
 <div className="grid-table-body-row" key={tractor._id}>
 <div className="grid-table-body-row-col">
 {tractor.registrationNumber}
 </div>
 <div className="grid-table-body-row-col">
 {tractor.manufacturer}
 </div>
 <div className="grid-table-body-row-col">
 {tractor.model}
 </div>
 <div className="grid-table-body-row-col">
 {tractor.actualMileage}
 </div>
 </div>)
 }
 </div>
 </div>
 <PaginationBar total={this.state.tractors.total} current={this.state.currentPage}
 pageSize={Number(this.state.pageParams.limit)}
 onShowSizeChange={this.changePage.bind(this)}>
 </PaginationBar>
 {
 this.isLoading && <span>Loading....</span>
 }
 </div>
 )
 }
}
export default TractorList;

pagination.js

import React, {Component} from 'react';
import {GlobalConst} from '../../constants/global';
import PropTypes from 'prop-types';
import ReactPaginate from 'react-paginate';
import './pagination.css';
class PaginationBar extends Component {
 static propTypes = {
 current: PropTypes.number,
 total: PropTypes.number,
 pageSize: PropTypes.number,
 onShowSizeChange: PropTypes.func
 };
 static defaultProps = {
 current: GlobalConst.defaultPageNr,
 total: 0,
 pageSize: GlobalConst.defaultPageLim
 };
 perPageOptions = [10, 25, 50, 100];
 constructor(props) {
 super(props);
 this.state = {
 pageSize: null
 }
 }
 componentDidMount() {
 // copy props to state, so we can use it (state) in this component
 const pageSize = this.props.pageSize;
 this.setState({
 pageSize: pageSize
 })
 }
 onShowSizeChange = (data) => {
 // when we change perPage drop-down
 this.setState({pageSize: data.target.value});
 this.props.onShowSizeChange(0, data.target.value);
 }
 onChange = (data) => {
 // when we change current page (pagination)
 this.props.onShowSizeChange(data.selected, this.props.pageSize);
 }
 render() {
 return (
 <nav className="d-flex justify-content-center position-relative pagination">
 <div className="pagination-per-page">
 <div className="form-group row align-items-center">
 <label htmlFor="perPageSelect" className="col-12 col-lg-6 pr-0 m-0">Per page:</label>
 <div className="col-12 col-lg-6">
 <select className="form-control" id="perPageSelect"
 onChange={this.onShowSizeChange}
 value={this.props.pageSize}>
 {
 this.perPageOptions.map((option, index) =>
 <option key={index}>{option}</option>)
 }
 </select>
 </div>
 </div>
 </div>
 <ReactPaginate previousLabel={"«"}
 nextLabel={"»"}
 breakLabel={<a href="" className="page-link">...</a>}
 breakClassName={"page-item disabled"}
 initialPage={this.props.current}
 forcePage={this.props.current}
 pageCount={Math.ceil(this.props.total / this.state.pageSize)}
 marginPagesDisplayed={2}
 pageRangeDisplayed={4}
 onPageChange={this.onChange}
 disableInitialCallback={true}
 containerClassName={"pagination"}
 activeClassName={"active"}
 pageClassName={"page-item"}
 pageLinkClassName={"page-link"}
 previousClassName={"page-item"}
 nextClassName={"page-item"}
 previousLinkClassName={"page-link"}
 nextLinkClassName={"page-link"}/>
 </nav>
 )
 }
}
export default PaginationBar;

tableHeader.js

import React, {Component} from 'react';
import PropTypes from 'prop-types';
import './tableHeader.css';
class TableHeader extends Component {
 state = {
 sortOrder: true,
 isActive: false
 };
 static propTypes = {
 text: PropTypes.string,
 field: PropTypes.string,
 isActive: PropTypes.bool,
 sortOrder: PropTypes.bool,
 onSortingChanged: PropTypes.func
 };
 static defaultProps = {
 text: '',
 sortOrder: undefined
 };
 componentDidMount() {
 // init our data
 this.setState({
 isActive: this.props.isActive,
 sortOrder: !this.props.sortOrder
 });
 }
 onSorted = () => {
 // get&set our sorting: field & way of sorting
 this.setState({
 isActive: true,
 sortOrder: !this.state.sortOrder
 });
 let sorted;
 this.state.sortOrder ? sorted = 'asc' : sorted = 'desc';
 this.props.onSortingChanged(this.props.field, sorted);
 }
 renderArrow() {
 // render sorting arrow: asc/desc
 if (this.props.isActive) {
 return this.state.sortOrder ? <span>↓</span> : <span>↑</span>
 }
 }
 render() {
 return (
 <div className={"table-header"} onClick={this.onSorted}>
 <span>{this.props.text}</span>
 {this.renderArrow()}
 </div>
 )
 }
}
export default TableHeader;
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jun 12, 2018 at 7:54
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Yeah you're definitely off to a good start. Here's how I think about state and props. Props are like parameters - they're passed into the component (duh). State is for data that was not passed into the component as props - often times this is data returned from AJAX calls. There's only one good reason I've seen to ever mix the two (see #3 below).

The other thing to consider is the component's lifecycle. If there's no asynchronous work, you should be able to setup all the state in the constructor. If you need to do something asynchronously, your component needs to support two variants of itself: one without data, and one with data.

For the more complex scenario, set the state for the first variant in the constructor, trigger the async work in componentDidMount(), then setState() when you have all the data you need. This should result in one two renders at most - the initial mount and the update.

Good stuff:

  1. Data fetching logic is in a service, and not baked into your component – this code can be reused if you switch from React to something else.
  2. Loading states (I start with this).

Some things that could be improved:

  1. AJAX Promise tracking (TractorList's isLoading) is state - put it in this.state; otherwise, your component might not update when you expect it.
  2. Parsing query string in TractorList could be moved to a function that simply returns state in an appropriate format – or you could do this at a higher level and just pass the result in as props to TractorList.
  3. Don't copy props to state like in pagination's componentDidMount(), because it leads to split-brain. The only valid use case I've seen is when you need to allow state and props to temporarily diverge, but still have the ability to roll-back to the previous state (e.g. a form in a modal where the user can save or cancel).
  4. Calling setState() and nothing else in TableHeader's componentDidMount(). This results in an unnecessary update – you could just put this in the constructor and assign to this.state directly from props.
  5. Error handling – AJAX calls fail all the time. To build a resilient app, you'll need to do crazy stuff, or leverage Error Boundaries. This might introduce additional state, and it's good to think through this before your component gets too complex.
answered Nov 5, 2018 at 6:32
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.