2
\$\begingroup\$

How can I improve this code? I am a beginner learning react.

This is a UI for managing banners. Users upload the image for the banner and specify a start date and end date between which the banner should be displayed. This UI gives the user a way to uplod new banners as well as delete them or change the image/dates for existing ones.

All suggestions are appreciated, including anything related to bugs, code style, organization, efficiency, libraries, or other.

The code is running here: http://hoday.mods.jp/public_html/bannerdisplay/react/index.html

app.js

import React from 'react';
import ReactDOM from 'react-dom';
import {Table} from 'components/table.js';
import {DataService} from 'js/dataservice.js';

let dataService = new DataService();

let config = {
    dataService: dataService,
    networkErrorMsg: "There has been a network error. Please try again later"
};

ReactDOM.render(
  <Table config={config}/>,
  document.getElementById('root')
);

components/table.js

import React from 'react';

class BannerDisplay extends React.Component {

    render() {

        return (
            <div className={['tr',this.props.isBeingHovered && 'row-hover'].join(' ')}>
                <div className="td td-image">
                    <img src={this.props.bannerInfo.src} />
                </div>
                <div className="td td-startDate">
                    {this.props.bannerInfo.startDate}
                </div>
                <div className="td td-endDate">
                    {this.props.bannerInfo.endDate}
                </div>
                <div className="td td-buttonGroup">
                    {this.props.isBeingHovered && 
                        <div>
                            <div className="btn btn-primary"
                                type="button" 
                                onClick={this.props.onClickEdit}>
                                Edit
                            </div>
                            <div className="btn btn-danger"
                                type="button" 
                                onClick={this.props.onClickDelete}
                                data-toggle="modal" 
                                data-target="#exampleModal">
                                Delete
                            </div>
                        </div>
                    }
                </div>          
            </div>
        );
    }       


}

class BannerForm extends React.Component {

    constructor(props) {
        super(props);
        this.state = {
            startDate: this.props.bannerInfo.startDate,
            endDate: this.props.bannerInfo.endDate,
            src: this.props.bannerInfo.src,
            hasError: false,
            errorMsg: ''

        };

        this.handleSubmit = this.handleSubmit.bind(this);
        this.handleFinishLoadFile = this.handleFinishLoadFile.bind(this);
        this.handleChange = this.handleChange.bind(this);

    }   

    handleSubmit(event) {
        event.preventDefault();
        let error = this.props.onClickSubmit(this.props.bannerInfo.bannerId, this.state.startDate, this.state.endDate, this.state.src)
        if (error) {
            this.setState({
                hasError: true,
                errorMsg: this.props.config.networkErrorMsg
            });
        }
    }

    handleChange(event) {
        const name = event.target.name;

        if (name == 'startDate' || name == 'endDate' ) {
            this.setState({
              [name]: event.target.value
            });             
        } else if (name == 'bannerImage') {
            var fileList = this.fileInput.files; // FileList object
            if (fileList && fileList[0]) {              
                var reader = new FileReader();
                reader.onload = this.handleFinishLoadFile;
                reader.readAsDataURL(fileList[0]);

            }           
        }   
    }

    handleFinishLoadFile(event) {
        let fileData = event.target.result;
        this.setState({
            src: fileData
        });         
    }

    render() {

        return (
        <form onSubmit={this.handleSubmit}>
            <div
                className="tr row-edit"  >
                <div className="td td-image">
                    <img src={this.state.src || 'dummy.jpg'} />
                    <input 
                        name="bannerImage"                  
                        type="file" 
                        accept="image/*"
                        ref={input => {
                            this.fileInput = input;
                        }}
                        onChange={this.handleChange} />
                </div>
                <div className="td td-startDate">
                    <input 
                        name="startDate"
                        type="text" 
                        value={this.state.startDate} 
                        onChange={this.handleChange} />
                </div>
                <div className="td td-endDate">
                    <input 
                        name="endDate"
                        type="text" 
                        value={this.state.endDate} 
                        onChange={this.handleChange} />
                </div>
                <div className="td td-buttonGroup">
                    <button
                        className="btn btn-danger"
                        type="button"
                        onClick={this.props.onClickCancel}>Cancel</button>              
                    <button
                        className="btn btn-primary"
                        type="submit">Submit</button>
                    {this.state.hasError &&
                        <div className="alert alert-danger">{this.state.errorMsg}</div>
                    }
                </div>              
            </div>
        </form>
        );
    }   

}


class RowExisting extends React.Component {

    constructor(props){
        super(props);   
        this.state = {
            isBeingHovered:false,
            isDeleteModalVisible: false,
        };

        this.handleClickSubmit = this.handleClickSubmit.bind(this);
        this.handleClickDelete = this.handleClickDelete.bind(this);
        this.handleClickEdit = this.handleClickEdit.bind(this);
        this.handleClickCancel = this.handleClickCancel.bind(this);
        this.handleClickModalCancel = this.handleClickModalCancel.bind(this);
        this.handleClickModalDelete = this.handleClickModalDelete.bind(this);


        this.handleMouseEnter = this.handleMouseEnter.bind(this);
        this.handleMouseLeave = this.handleMouseLeave.bind(this);


    }

    handleMouseEnter() {
        if (this.props.isHoverable) {
            this.setState({
              isBeingHovered: true
            });
        }
    }

    handleMouseLeave() {
        if (this.props.isHoverable) {
            this.setState({
              isBeingHovered: false
            });
        }
    }   

    handleClickSubmit(bannerId, startDate, endDate, fileData) {
        this.props.onClickSubmit(bannerId, startDate, endDate, fileData)


    }

    handleClickCancel() {
        this.props.onClickCancel();
    }   

    handleClickDelete() {
        this.setState({
            isDeleteModalVisible: true
        });     
    }

    handleClickModalDelete() {
        this.setState({
            isDeleteModalVisible: false
        });
        this.props.onClickDelete(this.props.bannerInfo.bannerId);       
    }

    handleClickModalCancel() {
        this.setState({
            isDeleteModalVisible: false
        });     
    }   


    handleClickEdit() {
        this.props.onClickEdit(this.props.bannerInfo.bannerId);
    }

    render() {

        let bannerForm = (
            <BannerForm 
                config={this.props.config}
                bannerInfo={this.props.bannerInfo}  

                onClickCancel={this.handleClickCancel}
                onClickSubmit={this.handleClickSubmit}
            />
        );

        let bannerDisplay = (
            <BannerDisplay 
                config={this.props.config}
                bannerInfo={this.props.bannerInfo}  

                onClickEdit={this.handleClickEdit}
                onClickDelete={this.handleClickDelete}
                isBeingHovered={this.state.isBeingHovered}
            />      
        );

        return (
            <div
                onMouseEnter={this.handleMouseEnter} 
                onMouseLeave={this.handleMouseLeave} >
                {this.props.isBeingEdited ? bannerForm : bannerDisplay}
                {this.props.hasError &&
                    <div className="alert alert-danger">
                        {this.props.config.networkErrorMsg}
                    </div>
                }
                {this.state.isDeleteModalVisible &&
                    <div>
                        <DeleteModal 
                            config={this.props.config}
                            onClickDelete={this.handleClickModalDelete}
                            onClickCancel={this.handleClickModalCancel}
                        />

                    </div>
                }
            </div>
        );

    }
}

class DeleteModal extends React.Component {

        constructor(props) {
            super(props);
        }

    render() {

        const backdropStyle = {
          position: 'fixed',
          top: 0,
          bottom: 0,
          left: 0,
          right: 0,
          backgroundColor: 'rgba(0,0,0,0.3)',
          padding: 50
        };

        const modalStyle = {
          backgroundColor: '#fff',
          borderRadius: 5,
          maxWidth: 500,
          minHeight: 300,
          margin: '0 auto',
          padding: 30
        };

        return (
          <div className="backdrop" style={backdropStyle}>
            <div className="deletemodal" style={modalStyle}>
              <div className="modalbody">
                <div>
                    Delete?
                </div>
              </div>

              <div className="modalfooter">
                <button className="btn btn-secondary" onClick={this.props.onClickCancel}>
                  Cancel
                </button>
                <button className="btn btn-danger" onClick={this.props.onClickDelete}>
                  Delete
                </button>           
              </div>
            </div>
          </div>
        );
    }       
}


class RowNew extends React.Component {

    constructor(props) {

        super(props);

        this.handleClickSubmit = this.handleClickSubmit.bind(this);
        this.handleClickCancel = this.handleClickCancel.bind(this);
        this.handleClickAdd     = this.handleClickAdd.bind(this);

    }

    handleClickSubmit(bannerId, startDate, endDate, fileData) {
        this.props.onClickSubmit(bannerId, startDate, endDate, fileData);
    }

    handleClickCancel() {
        this.props.onClickCancel();
    }

    handleClickAdd() {
        this.props.onClickAdd();
    }

    render() {

        return (
            <div>
                {this.props.isBeingEdited ? (
                    <BannerForm 
                        config={this.props.config}
                        bannerInfo={{bannerId:"", src:"", startDate:"", endDate:""}}
                        onClickCancel={this.handleClickCancel}
                        onClickSubmit={this.handleClickSubmit} />
                ) : (
                    <div className="container-newbutton">
                        <button 
                            type="button" 
                            className="btn btn-success" 
                            onClick={this.handleClickAdd}>
                            New
                        </button>
                    </div>
                )}
                {this.props.hasError &&
                    <div className="alert alert-danger">
                        {this.props.config.networkErrorMsg}
                    </div>
                }
            </div>
        );
    }

}


export class Table extends React.Component {

    constructor(props) {
        super(props);
        this.state = {
            banners: [], 
            isBeingEdited:  false,
            keyOfRowBeingEdited: 0,
            hasErrorFetchingRows: false,
            hasErrorCreatingRow: false,
            hasErrorUpdatingRow: false,
            hasErrorDeletingRow: false,
            errorMsg: '',
        };

        this.handleClickCancel = this.handleClickCancel.bind(this);
        this.handleClickAdd = this.handleClickAdd.bind(this);
        this.handleClickDelete = this.handleClickDelete.bind(this);
        this.handleClickSubmit = this.handleClickSubmit.bind(this);
        this.handleClickEdit  = this.handleClickEdit.bind(this);

    }

    handleClickCancel() {
        this.setState({
            isBeingEdited: false,
            hasErrorUpdatingRow: false,
            hasErrorCreatingRow: false,
            hasErrorDeletingRow: false
        });     
    }

    handleClickAdd(){
        this.setState({
            isBeingEdited: true,
            keyOfRowBeingEdited: '',
            hasErrorDeletingRow: false
        });
    }

    handleClickEdit(keyOfRowBeingEdited) {
        this.setState({
            isBeingEdited: true,
            keyOfRowBeingEdited: keyOfRowBeingEdited,
            hasErrorDeletingRow: false
        });     
    }

    handleClickSubmit(bannerId, startDate, endDate, fileData) {
        if (bannerId == '') {
            this.create(startDate, endDate, fileData);
        } else {
            this.update(bannerId, startDate, endDate, fileData);
        }
    }

    create(startDate, endDate, fileData){
        this.props.config.dataService.create(startDate, endDate, fileData)
        .then(bannerIdFromServer => {
            this.setState((prevState, props)=> {
                prevState.banners.push({
                    bannerId: bannerIdFromServer.toString(),    
                    src: fileData,                  
                    startDate: startDate,
                    endDate: endDate,
                });
                return {
                    banners: prevState.banners,
                    isBeingEdited: false,
                    hasErrorCreatingRow: false
                };

            });

        })
        .catch(msg => {
            console.log('error creating new');
            this.setState({
                hasErrorCreatingRow: true
            });
        });     
    }

    update(bannerId, startDate, endDate, fileData){
        this.props.config.dataService.update(bannerId, startDate, endDate, fileData)
        .then(msg => {

            this.setState((prevState, props)=> {
                let index = prevState.banners.findIndex(banner => (banner.bannerId == bannerId));       
                prevState.banners[index].startDate = startDate;
                prevState.banners[index].endDate = endDate;
                prevState.banners[index].src = fileData;
                return {
                    banners: prevState.banners,
                    isBeingEdited: false,
                    hasErrorUpdatingRow: false
                };
            });         
        })
        .catch(msg => {
            console.log('error updating');
            this.setState({
                hasErrorUpdatingRow: true
            });
        });     
    }

    handleClickDelete(bannerId) {
        this.props.config.dataService.delete(bannerId)
        .then(msg => {

            this.setState((prevState, props) => {
                let bannerIndex = prevState.banners.findIndex(banner => (bannerId == banner.bannerId));
                prevState.banners.splice(bannerIndex, 1);
                return {
                    banners: prevState.banners,
                    hasErrorDeletingRow: false,
                    keyOfRowBeingEdited: bannerId
                };
            });         
        })
        .catch(msg => {
            console.log('error deleting');
            this.setState({
                hasErrorDeletingRow: true,
                keyOfRowBeingEdited: bannerId
            });         
        });     
    }   

    componentDidMount() {
        this.props.config.dataService.get()
        .then(banners => {
            this.setState({banners});       
        })
        .catch(msg => {
            console.log('error fetching data');
            this.setState({
                hasErrorFetchingRows: true,
                errorMsg: this.props.config.networkErrorMsg
            });     

        });

    }


    render() {

        let isRowNewVisible = !(this.state.isBeingEdited && this.state.keyOfRowBeingEdited != '');

        const rows = this.state.banners.map(banner =>
            <RowExisting
                config={this.props.config}
                key={banner.bannerId}
                bannerInfo={banner}

                onClickSubmit={this.handleClickSubmit}
                onClickDelete={this.handleClickDelete}

                onClickEdit={this.handleClickEdit}
                onClickCancel={this.handleClickCancel}

                hasError={(this.state.hasErrorUpdatingRow || this.state.hasErrorDeletingRow)  && this.state.keyOfRowBeingEdited == banner.bannerId}
                isHoverable={!this.state.isBeingEdited}
                isBeingEdited={this.state.isBeingEdited && this.state.keyOfRowBeingEdited == banner.bannerId} />
        );

        return (
            <div>
              <div className="table">
                <div className="table-head">
                  <div className="tr">
                    <div className="th td-image">Image</div>
                    <div className="th td-startDate">Start date</div>
                    <div className="th td-endDate">End date</div>
                    <div className="th td-buttonGroup"></div>
                  </div>
                </div>
                <div className="table-body">
                    {this.state.hasError ?
                        <div className="alert alert-danger" role="alert">{this.state.errorMsg}</div>
                    :
                        <div>{rows}</div>
                    }
                </div>
              </div>
              <div className="table">
                <div className="table-body">
                    {isRowNewVisible &&
                        <RowNew 
                            config={this.props.config}
                            onClickSubmit={this.handleClickSubmit}
                            onClickAdd={this.handleClickAdd}
                            onClickCancel={this.handleClickCancel}
                            hasError={this.state.hasErrorCreatingRow}
                            isBeingEdited={this.state.isBeingEdited && this.state.keyOfRowBeingEdited == ''}
                        />
                    }
                </div>
              </div>


            </div>        
        );

    }

}

js/dataservice.js (stub class just for testing)

export class DataService {

    constructor() {
        this.bannerIdFromServer = 99;
    }

    get() {

        return new Promise(function(resolve, reject) {
            const BANNERS = [
              {bannerId: '1', src: './uploads/banner1.jpg', startDate: '2020-04-40', endDate: '2020-10-22'},
              {bannerId: '2', src: './uploads/banner2.png', startDate: '2018-01-14', endDate: '2018-01-15'},
              {bannerId: '3', src: './uploads/banner3.jpg', startDate: '2018-01-14', endDate: '2018-01-15'},

            ];
            resolve(BANNERS);
            //reject();
        });
    }

    create(startDate, endDate, fileData) {
        let bannerIdFromServer = this.bannerIdFromServer++;
        return new Promise(function(resolve, reject) {
            resolve(bannerIdFromServer);
            //reject();
        });
    }

    update(bannerId, startDate, endDate, fileData) {
        return new Promise(function(resolve, reject) {
            resolve();
            //reject();
        });     

    }

    delete(bannerId) {
        return new Promise(function(resolve, reject) {
            resolve();
            //reject();
        });     
    }   

}

index.html

<!DOCTYPE html>
<!--[if lte IE 9]>
<html class="ie" lang="en">
<![endif]-->
<!--[if gt IE 9]><!-->
<html lang="en" class="no-js cookie_used_true">
<!--<![endif]-->

<head>

  <meta charset="UTF-8">

  <title>Banner manager</title>

    <script src="https://code.jquery.com/jquery-3.2.1.slim.min.js" integrity="sha384-KJ3o2DKtIkvYIK3UENzmM7KCkRr/rE9/Qpg6aAZGJwFDMVNA/GpGFF93hXpG5KkN" crossorigin="anonymous"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.12.9/umd/popper.min.js" integrity="sha384-ApNbgh9B+Y1QKtv3Rn7W3mgPxhU9K/ScQsAP7hUibX39j7fakFPskvXusvfa0b4Q" crossorigin="anonymous"></script>
    <script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/js/bootstrap.min.js" integrity="sha384-JZR6Spejh4U02d8jOt6vLEHfe/JQGiRRSQQxSfFWpi1MquVdAyjUar5+76PVCmYl" crossorigin="anonymous"></script>
    <script defer src="https://use.fontawesome.com/releases/v5.0.10/js/all.js" integrity="sha384-slN8GvtUJGnv6ca26v8EzVaR9DC58QEwsIk9q1QXdCU8Yu8ck/tL/5szYlBbqmS+" crossorigin="anonymous"></script>
    <script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/systemjs/0.19.31/system.js"></script>


    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" />
    <link rel="stylesheet" href="style.css" />


    <script>
        System.config({
                transpiler: 'babel',
                baseURL: '',
                map: {
                    babel: 'https://cdnjs.cloudflare.com/ajax/libs/babel-core/5.8.34/browser.min.js',
                    react: 'https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.js',
                    'react-dom': 'https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.js'
                }
        });

        // load application
        System.import('./js/dataservice.js');   
        System.import('./components/table.js');
        System.import('./app.js');

    </script>

</head>

<body>

    <div class="container">
        <div id="root">
        </div>
    </div>

</body>

style.css

body {
    padding: 5px
}

.table {
    display: table;
}

.tr {
    display:flex;
    align-items:stretch;
}

.th {
    font-weight: bold;
    background-color: #AACCFF;
    height: 4rem;
}

.td, .th {
    display: flex;
    align-items: center;
    justify-content: center;
}   

.td {
    border-bottom: solid 1px #999999;
}

.td-image {
    flex: 2 0 35%;
}   

.td-startDate,
.td-endDate     {
    flex: 1 0 20%;
}

.td-buttonGroup {
    flex: 0 0 200px;
}

.td-image img {
    min-height: 3rem;
}       

button {
    margin-top: 1em;
}

img {
    height: 100px;
}

.row-hover,
.row-edit {
    background-color: #CCCCFF;
}

.btn {
    line-height: 2em;
    margin: 4px;
}

.container-newbutton {
    text-align: right;
}

.deletemodal {
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
}

.modalbody {
    flex: 1 1 100%;
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;            
}

.modalfooter {
    flex: 0 0 auto;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Could you add a bit more description of what this code does? (i.e. a rough outline of the functional requirements). With more information about the code and its real-world purpose, you'll get more and better reviews. \$\endgroup\$ – Toby Speight Apr 20 '18 at 8:08
  • \$\begingroup\$ Thanks. This is a UI for managing banners. Users upload the image for the banner and specify a start date and end date between which the banner should be displayed. This UI gives the user a way to uplod new banners as well as delete them or change the image/dates for existing ones. \$\endgroup\$ – circularsoggy Apr 25 '18 at 16:43

Your Answer

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

Browse other questions tagged or ask your own question.