A
A
AlexKindGeek2018-10-02 23:40:42
React
AlexKindGeek, 2018-10-02 23:40:42

Are the components written correctly??

Hello. Guys, I need a side view of my code.
Are the components written correctly??
Perhaps there are points that are worth paying attention to?

UserListComponent
import React, { Component, Fragment } from "react";
import { connect } from "react-redux";

/**Components */
import InfiniteScroll from "react-infinite-scroll-component";
import { BeatLoader } from "react-spinners";
import User from "./User";

/**Actions */
import { getListOfUsers, clearData, setUserChat } from "../../../actions";

/**Configs, Utils */

/**Material UI */

class UserList extends Component {
  state = {
    per_page: 15
  };

  componentDidMount() {
    this.getChatsUser();
  }

  /**Called when we change chat type (all, unread, read etc.) */
  componentDidUpdate(prevProps) {
    const { clearData } = this.props;
    if (prevProps.params !== this.props.params) {
      clearData();
      this.getChatsUser();
    }
  }

  fetchMoreData = () => {
    this.getChatsUser();
  };

  /**
   * Get query of params from props and called api action for getting users.
   * When fetchMoreData is called, we should increase per_page and
   * override userData at reducer (getListOfUsers action)
   */
  getChatsUser = () => {
    const { getListOfUsers, params } = this.props;
    const { per_page } = this.state;
    const query = Object.assign({}, params, { per_page });
    getListOfUsers(query).then(() => {
      this.setState({
        per_page: this.state.per_page + 5
      });
    });
  };

  render() {
    const {
      chatReducer: { userData },
      setUserChat
    } = this.props;
    return (
      <Fragment>
        <InfiniteScroll
          dataLength={userData.length}
          next={this.fetchMoreData}
          hasMore={true}
          height={520}
          loader={
            <BeatLoader
              className="chat-loader"
              sizeUnit="px"
              size={12}
              color="#123abc"
            />
          }
          endMessage={
            <p style={{ textAlign: "center" }}>
              <b>Yay! You have seen it all</b>
            </p>
          }
        >
          {userData.map((row, index) => (
            <User setUserForChat={setUserChat} key={row.id} userData={row} />
          ))}
        </InfiniteScroll>
      </Fragment>
    );
  }
}

const mapDispatchToProps = {
  getListOfUsers,
  clearData,
  setUserChat
};

const mapStateToProps = (state) => ({
  chatReducer: state.chatReducer
});

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

User Component
import React from "react";
import moment from "moment";

class User extends React.Component {
  formatTime = (data) => {
    if (data) {
      const started = moment.unix(data).format("HH:mm MM/DD/YYYY");
      const finished = moment().format("HH:mm MM/DD/YYYY");
      let minutes = moment(finished).diff(moment(started), "minutes");
      const days = moment(finished).diff(moment(started), "days");
      const hours = Math.floor(minutes / 60);
      minutes %= 60;
      if (days > 0) {
        return moment.unix(data).format("MM/DD/YYYY");
      }
      if (hours > 0) {
        return `${hours}h`;
      }
      return `${minutes}m`;
    }
  };

  render() {
    const {
      userData,
      userData: { user_id, message_created_at, message, unread_amount },
      setUserForChat
    } = this.props;
    return (
      <div
        className="user-data-container"
        onClick={() => setUserForChat(userData)}
      >
        <div className="user-cell-container">
          <div className="user-id-container">User # {user_id}</div>
          <div className="message-time-container">
            {this.formatTime(message_created_at)}
          </div>
        </div>
        <div className="user-cell-container">
          <div className="last-message-container">
            {message || "No messages"}
          </div>
          {unread_amount > 0 ? (
            <div className="unread-message-container">{unread_amount}</div>
          ) : (
            ""
          )}
        </div>
      </div>
    );
  }
}

export default User;

Answer the question

In order to leave comments, you need to log in

2 answer(s)
A
Anton Spirin, 2018-10-03
@AlexKindGeek

1. formatTime is a helper, I will not waste time analyzing the quality of its implementation, I will only say that it does not belong in a component. Transfer to the lib, utils folder or whatever you call it, import where you need it and use it.
2. For now, let's close our eyes to point 1. formatTime is written by an arrow function, but it is not passed anywhere and the this keyword is not used in it - it was pointless to define it with an arrow function. Study the question and try to understand why they are used.
The same with getChatsUser - the function is not passed anywhere.
3. Never use ternarks if the alternative case is null. Change to:

render() {
  /* ... */
  const hasUnreadMessages = unread_amount > 0;
  
  return (
    <div>
      {hasUnreadMessages && (
        <div className="unread-message-container">{unread_amount}</div>
      )}
    </div>
  );
}

4. Why are you passing all the reducer state to mapStateToProps? Pass only the data that the component needs:
const mapStateToProps = state => ({
  userData: userDataSelector(state),
});

Selectors are very useful. In real applications, they are often reused many times, and if the store structure changes, you only need to change the selectors, instead of changing the implementations of mapStateToProps throughout the application. Also, they are often shorter.
Read about the reselect library.
5. Instead of:
more concise and easier to analyze:
6. Correct state update based on the previous one:
this.setState(prevState => ({
  per_page: prevState.per_page + 5,
}));

7. Using trailing comma is a very good practice, I advise you not to neglect it.
8. A more optimized option for lists, instead of defining arrow functions in the elements of each render:,
use data-attributes:
The implementation of setUserForChat will have to be changed:
setUserForChat = e => {
  const { id } = e.target.dataset;
  // some code with using id
}

A
abberati, 2018-10-03
@abberati

good tone - in ternary operators, instead of ""putting the last operandnull

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question