2

I have a simple React/Redux app that displays a list of cars based on my Rails API.

I'm trying to add a sort feature that alphabetizes the cars by their name.

While my variable orgArray is in fact alphabetized when I console.log it, my Redux dev tool says states are equal after clicking the sort button - therefore my UI isn't updated.

Here's my code:

import React, { Component } from 'react';
import { connect } from 'react-redux';
import CarCard from '../components/CarCard';
import CarForm from './CarForm';
import './Cars.css';
import { getCars } from '../actions/cars';
import { sortCar } from '../actions/cars';

Component.defaultProps = {
  cars: { cars: [] }
}

class Cars extends Component {
  constructor(props) {
    super(props)
      this.state = {
        cars: [],
        sortedCars: []
      };
  }

sortAlphabetically = () => {
    console.log("sort button clicked")
    const newArray = [].concat(this.props.cars.cars)
    const orgArray = newArray.sort(function (a,b) {
      var nameA = a.name.toUpperCase();
      var nameB = b.name.toUpperCase();
          if (nameA < nameB) {
            return -1;
          } else if (nameA > nameB) {
            return 1;
          } 
            return 0;
          }, () => this.setState({ cars: orgArray }))  
            console.log(orgArray)
            this.props.sortCar(orgArray);
          }

    componentDidMount() {
        this.props.getCars()
        this.setState({cars: this.props.cars})
        }


    render() {
        return (
        <div className="CarsContainer">
    <h3>Cars Container</h3> 
        <button onClick={this.sortAlphabetically}>Sort</button>
        {this.props.cars.cars && this.props.cars.cars.map(car => <CarCard key={car.id} car={car} />)}  
        {/* {this.state.cars.cars && this.state.cars.cars.map(car => <CarCard key={car.id} car={car} />)}           */}
        <CarForm />
    </div>
    );
  }
}

 const mapStateToProps = (state) => {
   return ({
    cars: state.cars
  })
}

const mapDispatchToProps = (dispatch) => {
  return {
    sortCar: (cars) => dispatch(sortCar(cars)),
    getCars: (cars) => dispatch(getCars(cars))
  }
}

export default connect(mapStateToProps, mapDispatchToProps)(Cars);

I would have guessed that mapStateToProps or adding the sortedCars: [] in my initial setState would have worked.

Essentially, my props are getting updated, but I need my state to be updated as well - though I'm not sure what I'm missing.

UPDATED:

Here's my action creator and Async action if it helps:

const sortCars = cars => {
 return {
    type: 'SORT_CARS',
    cars
  }
}

// Async Actions

export const sortCar = (cars) => {
  console.log(cars, 'cars object');
  return dispatch => {
    dispatch(sortCars(cars))
   }
}

UPDATE:

Here's the Reducer as well:

export default (state = {cars: []}, action) => {
switch(action.type) {
    case 'GET_CARS_SUCCESS':

    return Object.assign({}, state, {cars: action.payload})

    case 'CREATE_CAR_SUCCESS':

    return Object.assign({}, state, {cars: action.payload})

    case 'REMOVE_CAR;':
    return state.filter(car => car.id !== action.id)

    case 'SORT_CARS;':
    return Object.assign({}, state, { cars: action.payload})

    default:
        return state;
  }
}
James
  • 411
  • 1
  • 4
  • 18
  • Can you sort the array in your resolver? It seems unnecessary to pass the array in twice, once unsorted and once sorted. – Tom Mar 19 '19 at 12:42
  • Sorry, but I'm not sure what you mean by `resolver` – James Mar 19 '19 at 12:45
  • Sorry, the reducer – Tom Mar 19 '19 at 12:48
  • Wait, your code is confusing me. In the sort function, what's that close-bracket after `return 0;`? – Alesh Houdek Mar 19 '19 at 12:53
  • The closing bracket after `return 0;` should close out the `newArray.sort(function` line – James Mar 19 '19 at 12:55
  • @Tom are you suggesting that I put the sort function in the reducer? Or just add the `setState: orgArray` in it? – James Mar 19 '19 at 12:59
  • 1
    @James yeah I would make sure that the data going to your view is sorted, cleaned and ready to use. – Tom Mar 19 '19 at 13:22
  • I'm still confused by the indentation ... it looks like you're passing `() => this.setState({ cars: orgArray })` as a second parameter to the sort function, but sort only takes one parameter? – Alesh Houdek Mar 19 '19 at 14:57

4 Answers4

1

First, I think you don't actually need any state here, you can just update the store and show cars from props.

Also, I think the problem here is that you pass orgArray to this.props.sortCar(orgArray) which is an array instead of object with "cars" key and values inside.

Apart from that setState call is being declared as anonymous function instead of being actually executed after the sorting.

Make sure to indent your code.

Yosif Mihaylov
  • 108
  • 1
  • 9
  • Good thinking, I think that'll contribute to the solution, but my UI isn't updating. – James Mar 19 '19 at 13:24
  • @James I updated my comment, checkout the part about passing array instead of object with "cars" key, make sure that the reducer is doing what it's supposed to. – Yosif Mihaylov Mar 19 '19 at 13:34
  • Thank you, I appreciate your help! – James Mar 19 '19 at 13:40
  • `orgArray` is an array of objects, so maybe I should be mapping it somewhere? I'm not sure what next steps I should take – James Mar 19 '19 at 13:44
  • I think you should be passing a object here `this.props.sortCar({cars: orgArray})` but I can't be sure without seeing the reducer and the initial model/state/store, it would be helpful to include those in your question. – Yosif Mihaylov Mar 19 '19 at 13:54
  • Added Reducer and Actions/Creators. – James Mar 19 '19 at 13:58
  • 1
    You're dispatching an action of type `SORT_CAR` but in the reducer you're expecting `SORT_CARS;` type action (you should remove this semicolon from the name), this should be a problem also you should be passing a object `this.props.sortCar({cars: orgArray})` as I previously suggested. – Yosif Mihaylov Mar 19 '19 at 14:03
  • Fixed the 'S'. Where exactly should I pass that object? In the `return dispatch` of my async action? – James Mar 19 '19 at 14:06
  • 1
    You should remove the semicolon from the expected reducer action `case 'SORT_CARS' ` not `case 'SORT_CARS;'`, and you should simply call `this.props.sortCar({cars: orgArray})` instead of `this.props.sortCar(orgArray)` – Yosif Mihaylov Mar 19 '19 at 14:10
  • @James Would you mark this answer as the correct one if you think it helped to solve your problem, thanks :) – Yosif Mihaylov Mar 19 '19 at 15:59
0

Sort only have one argument (the sort function). And in your sortAlphabetically method setState (placed as a second argument of sort) is not being called. The method should be something like that (I didn't tested it)

sortAlphabetically = () => {
    console.log("sort button clicked")
    const newArray = this.props.cars.cars.slice();
    const orgArray = newArray.sort(function (a, b) {
        var nameA = a.name.toUpperCase();
        var nameB = b.name.toUpperCase();
        if (nameA < nameB) {
            return -1;
        } else if (nameA > nameB) {
            return 1;
        }
        return 0;
    });
    this.setState({ cars: orgArray }, () => {
        console.log(orgArray);
        this.props.sortCar(orgArray);
    });
}
MiguelML
  • 164
  • 1
  • 5
0

Got a solution that just uses the local state instead of the Redux store (not ideal, but fulfills this feature).

Adding async to my componentDidMount along with the await waits to update the data until the local state has changed, which then reflects in my UI.

Thanks everyone for your help.

import React, { Component } from 'react';
import { connect } from 'react-redux';
import CarCard from '../components/CarCard';
import CarForm from './CarForm';
import './Cars.css';
import { getCars } from '../actions/cars';
import { sortCar } from '../actions/cars';

Component.defaultProps = {
  cars: { cars: [] }
}

class Cars extends Component {
  constructor(props) {
  super(props)
  this.state = {
    cars: [],
    sortedCars: []
  };
}

sortAlphabetically = () => {
    console.log("sort button clicked")
    const newArray = [].concat(this.props.cars.cars)
    const orgArray = newArray.sort(function (a,b) {
          var nameA = a.name.toUpperCase();
          var nameB = b.name.toUpperCase();
              if (nameA < nameB) {
                return -1;
              } else if (nameA > nameB) {
                return 1;
              } 
               return 0;
              })  
            console.log(orgArray)
            this.props.sortCar(orgArray);
            this.setState({ cars: {cars: orgArray} })
}

async componentDidMount() {
    await this.props.getCars()
    this.setState({cars: this.props.cars})
}

render() {
    return (
    <div className="CarsContainer">
        <h3>Cars Container</h3> 
        <button onClick={this.sortAlphabetically}>Sort</button>
        {this.state.cars.cars && this.state.cars.cars.map(car => <CarCard key={car.id} car={car} />)}          
        <CarForm />
    </div>
    );
  }
}

 const mapStateToProps = (state) => {
  return ({
    cars: state.cars
  })
}

const mapDispatchToProps = (dispatch) => {
  return {
    sortCar: (cars) => dispatch(sortCar(cars)),
    getCars: (cars) => dispatch(getCars(cars))
  }
}

export default connect(mapStateToProps, mapDispatchToProps)(Cars);
James
  • 411
  • 1
  • 4
  • 18
0

You should not use setState(). setState() does not immediately mutate this.state but creates a pending state transition. this.state after calling this method can potentially return the existing value. so here is how to handle sorting.

NOTE that by default, the sort() method sorts the values as strings in alphabetical and ascending order. so u dont need to define any compare function.

1-u can define another action and have reducer handle the sorting. the thing is here u have to make sure u do not mutate the state so use slice()

case 'SORT_CARS':
  return state.slice().sort()

2- u can define a function and pass the state as arguments and return the sorted state.

const mapStateToProps = (state) => {
  return ({
    cars: state.cars.sort()
  })
}
Yilmaz
  • 35,338
  • 10
  • 157
  • 202