4
\$\begingroup\$

The requirements are as follows:

Create a Flight app service using React that does the following:

Login page:

  • When entering username: “user” and password: “password”, the user will be routed to the app page.
  • otherwise - an error message should be presented.

Flight list page:

  • Table of flights with the fields: from,​ to​, departureTime​, landingTime​, price.​
  • If there are no flights, display “No flights listed” in the first table row.
  • Add the ability to filter the flights by flight destination

Add new flight page with a form:

  • On submit, the new flight will be added to the list.

My design:

I separated components into App, Login and FlightList.

App: Contained the routes, and the handleLogin callback passed to the Login Component.

Login: Contained inputs for user, pass. Each change in inputs updated the components state and when clicking the button I invoked the handleLogin cb and sent it the state.

FlightList: After component mounts I call a fake api returning a promise with array of flights, and set state with the array. then map over it to render a table. I also save another copy of the array in order to allow filtering by flight destination. The component has a text input, that when changed filters the copy of the array and setting it as the main array. This component also has a Modal that helps create new flight.

My code:

index.js:

import React from 'react';
import ReactDOM from 'react-dom';
import './index.css';
import App from './App';
import { BrowserRouter } from 'react-router-dom';
import registerServiceWorker from './registerServiceWorker';

ReactDOM.render(
    <BrowserRouter>
        <App />
    </BrowserRouter>
    , document.getElementById('root'));
registerServiceWorker();

App.js:

import React, { Component } from 'react';
import './App.css';
import Login from "./Components/Login/Login";
import FlightsList from "./Components/FlightsList/FlightsList";
import { Link, Route, Switch } from 'react-router-dom';
import { withRouter } from 'react-router-dom';

class App extends Component {
    handleLogin = (loginObject) => {
        if( loginObject.userName === 'user' && loginObject.password === 'pass') {
            this.props.history.push({
                pathname: '/flights',
            });
        }
    };

    render() {
        return (
                <Switch>
                    <Route exact path="/" component={() => <Login doLogin={this.handleLogin} />}/>
                    <Route path="/flights" component={FlightsList}/>
                </Switch>
        );
    }
}

export default withRouter(App);

Login.js

import React from 'react';
import PropTypes from 'prop-types';
import './Login.css';
import { withRouter } from 'react-router-dom';

class Login extends React.Component {

    static propTypes = {
        doLogin: PropTypes.func.isRequired,
    };

    handleChangedInput = (e) => {
        this.setState({
            [e.target.name]: e.target.value,
        });
    };

    sendLoginInfo = () => {
        this.props.doLogin(this.state);
    };

    render() {
        return(
            <div className="login-div">
                <h2>Login</h2>
                <div>
                    <label>User Name: </label>
                    <input name="userName" type="text" onChange={this.handleChangedInput}/>
                </div>
                <div>
                    <label>Password: </label>
                    <input name="password" type="password" onChange={this.handleChangedInput} />
                </div>
                <button onClick={this.sendLoginInfo} className="login-btn" name="button">{'Login'}</button>
            </div>
        );
    }
}

export default withRouter(Login);

FlightList.js:

import React, { Component } from 'react';
import './FlightsList.css';
import getFlights from '../../helpers/api';
import { Table, Button } from 'react-bootstrap';
import Modal from 'react-modal';
const customStyles = {
    content : {
        top                   : '50%',
        left                  : '50%',
        right                 : 'auto',
        bottom                : 'auto',
        marginRight           : '-50%',
        transform             : 'translate(-50%, -50%)'
    }
};

export default class FlightsList extends Component {

    state = {
        flights: null,
        originalFlights: null,
        modalIsOpen: false,
    };
    componentDidMount() {
        getFlights()
            .then( flights => {
                this.setState({
                    flights,
                    originalFlights: flights,
                });
            });
    }

    openModal = () => {
        this.setState({modalIsOpen: true});
    };

    closeModal = () => {
        this.setState({modalIsOpen: false});
    };

    renderFlights() {
        return (
                this.state.flights.map( flight => (
                    <tr key={flight.id}>
                        <td>{flight.from}</td>
                        <td>{flight.to}</td>
                        <td>{flight.departureTime}</td>
                        <td>{flight.landingTime}</td>
                        <td>{flight.price}</td>
                    </tr>
                ))
        );
    }

    rednerNoFlights() {
        return (
            <tr colSpan={5}>{'No Flights'}</tr>
        );
    }

    renderLoading() {
        return (
            <tr colSpan={5}><td>{'Loading'}</td></tr>
        );
    }

    handleKeyPress = (e) => {

        if(e.key == 'Enter'){
            if( e.target.value === '') {
                this.setState({
                    flights: this.state.originalFlights,
                });
            }
            else {
                this.setState({
                    flights: this.state.flights.filter(flight => flight.to === e.target.value),
                    originalFlights: this.state.flights,
                });
            }
        }
    };

    handleDetailsSubmit = (e) => {
        const newFlight = {
            id: this.state.flights.length,
            from: this.refs.from.value,
            to: this.refs.to.value,
            departureTime: this.refs.departure.value,
            landingTime: this.refs.landing.value,
            price: this.refs.price.value,
        };
        this.setState({
            flights: [...this.state.flights, newFlight],
            originalFlights: [...this.state.flights, newFlight],
            modalIsOpen: false,
        });
    };

    renderModal = () => {
        return (
            <Modal
                isOpen={this.state.modalIsOpen}
                onRequestClose={this.closeModal}
                style={customStyles}
                contentLabel="Example Modal"
            >
                <div>
                    <form className="form_style">
                        <p>
                            <label>{'from:'}</label>
                            <input type="text" ref="from" name="from" />
                        </p>
                        <p>
                            <label>{'to: '}</label>
                            <input type="text" ref="to" name="to" />
                        </p>
                        <p>
                            <label>{'Departure Time: '}</label>
                            <input type="text" ref="departure" name="departure_time" />
                        </p>
                        <p>
                            <label>{'Landing Time: '}</label>
                            <input type="text" ref="landing" name="landing_time" />
                        </p>
                        <p>
                            <label>{'Price: '}</label>
                            <input min="0" ref="price" type="number" name="price" />
                        </p>
                        <div className="button_div">
                            <Button onClick={this.handleDetailsSubmit} bsStyle="primary" bsSize="small">
                                {'Save'}
                            </Button>
                        </div>
                    </form>
                </div>
            </Modal>
        );
    };

    render() {
        return (
            <div>
                <h1>Flights</h1>
                <div>
                    <label>{'Filter: '}</label>
                    <input type="text" onKeyPress={this.handleKeyPress}/>
                </div>
                <div>
                    <button onClick={this.openModal} >{'Add Flight'}</button>
                </div>
                {this.renderModal()}
                <Table striped bordered condensed hover>
                    <thead>
                    <tr>
                        <th>from</th>
                        <th>to</th>
                        <th>Departure Time</th>
                        <th>Landing Time</th>
                        <th>Price</th>
                    </tr>
                    </thead>
                    <tbody>
                        {!this.state.flights ? this.renderLoading() : ( this.state.flights.length > 0 ? this.renderFlights() : this.rednerNoFlights() )}
                    </tbody>
                </Table>
            </div>
        );
    }
}

api.js:

export default function getFlights() {
    return new Promise((resolve, reject) => {
        setTimeout(() => resolve([
            {
                id: 0,
                from: 'Tel-Aviv',
                to: 'New-York',
                departureTime: 55,
                landingTime: 55,
                price: 300,
            },
            {
                id: 1,
                from: 'Tel-Aviv',
                to: 'Amsterdam',
                departureTime: 55,
                landingTime: 55,
                price: 300,
            },
            {
                id: 2,
                from: 'Tel-Aviv',
                to: 'New-York',
                departureTime: 55,
                landingTime: 55,
                price: 300,
            }
        ]), 2000);
    });
}

Some questions:

  • What would you do differently?
  • Is it a good practice to pass callback to the login component, or should the login component hold the handleLogin function itself?
  • Is it a good practice to store user and pass in the Login components state and send it as I did?
  • Would you create a add flight component containing the Modal and send it the handleDetailsSubmit function, instead of rendering it inside the flightList component?
  • Would you handle the no-flights and loading display differently?
  • How would you handle the filtering differently? I know it's not very efficient.
  • Would you handle routing differently?
  • I know I should have added some validations, but I didn't have time.
\$\endgroup\$
1
\$\begingroup\$

Is it a good practice to pass callback to the login component, or should the login component hold the handleLogin function itself?

According to the single responsibility principle: no. The App component defines the routing. It feels awkward that it also handles login logic. It seems to be doing too many different things. While the Login component seems perfectly suitable for handling everything login-related.

Is it a good practice to store user and pass in the Login components state and send it as I did?

This code looks needlessly tricky to me. It's not immediately clear what kind of values are stored to the state. I would just write it out explicitly:

onChange={(e) => this.setState({password: e.target.value})}

Or alternatively, drop onChange callbacks altogether and use refs to extract user/pass values when the Login button is clicked.

Would you create a add flight component containing the Modal and send it the handleDetailsSubmit function, instead of rendering it inside the flightList component?

Definitely. The FlightsList is already a pretty large component. I'd suggest splitting it into even more smaller components, like:

  • FlightRow
  • EmptyRow

Would you handle the no-flights and loading display differently?

The <tr> element has no colspan attribute. That attribute is for <td>. Also <tr> cannot contain text within itself, only inside <td>.

There's a pretty long line for deciding between these different displays:

{!this.state.flights ? this.renderLoading() : ( this.state.flights.length > 0 ? this.renderFlights() : this.rednerNoFlights() )}

Extract it to a method and use normal if-s to break it to multiple lines instead of using ?:.

How would you handle the filtering differently? I know it's not very efficient.

I would not worry about efficiency here. Rather I think the search is not very useful: you can only search by exact value of "to" field. Try to make it work so that you can filter by only writing part of the destination name, like "Ams" instead of "Amsterdam". Filtering on every keypress would also be nice.

Note that the filtering isn't applied to the newly created entries.

Would you handle routing differently?

Looks pretty straight-forward to me. Only minor things:

  • No need to apply withRouter() to Login component.
  • The imported Link component is unused.

Last, but not least

Check browser console for errors/warnings and fix them.

\$\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.