5
\$\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;
\$\endgroup\$
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.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.