3
\$\begingroup\$

I have put together a piece of what can be a much larger frontend in Reactjs.

I just put together the component that renders to the user a list of airports and hotels as well as a header component.

I feel confident about the JavaScript implementation, but what I would like some feedback is on my use of SCSS. I installed node-sass and this is how I implemented it.

LocationList.js:

renderLocation() {
    const filteredLocations = this.props.locations.filter(location => {
      return !location.name.match(/[A-Z0-9]+$/);
    });

    return filteredLocations.map(location => {
      if (location.airport_code) {
        return (
          <div key={location.id}>
            <div className="location">
              <h1>
                {location.name} ({location.airport_code})
              </h1>
              <div className="location-secondary-info">
                <span>
                  <i className="material-icons">airplanemode_active</i>
                  {location.description}
                </span>
              </div>
            </div>
          </div>
        );
      } else {
        return (
          <div key={location.id}>
            <div className="location">
              <h1>{location.name}</h1>
              <div className="location-secondary-info">
                <span>
                  <i className="material-icons">location_city</i>
                  {location.description}
                </span>
              </div>
            </div>
          </div>
        );
      }
    });
  }

  render() {
    return <div className="locations-container">{this.renderLocation()}</div>;
  }
}

Header.js:

const Header = () => {
  return (
    <header className="App-header">
      <div>
        <div>
          <section>
            <i className="material-icons">menu</i>
            <Link to="/" className="anchor">
              Silvercar
            </Link>
          </section>
          <div className="account">
            <section>
              <aside className="account">
                <Link to="/" className="pill anchor">
                  Sign Up | Login
                </Link>
                <Link to="/" className="help">
                  Help
                </Link>
              </aside>
            </section>
          </div>
        </div>
      </div>
    </header>
  );
};

App.scss:

@import "../variables.scss";

body {
  font-size: 14px;
}

.anchor {
  color: $link-color;
  text-decoration: none;
  font-weight: 500;
  font-size: 0.85em;
  text-transform: uppercase;
}

.App {
  display: flex;
  height: 100vh;
}

.App-header {
  border-bottom: 1px solid #eee;
}

.App-header div {
  max-width: 1200px;
}

.App-header div div {
  padding: 16px 8px;
}

.App-header div div section {
  flex: 1 1;
  display: flex;
  align-items: center;
}

section i {
  padding-right: 5px;
}

.App-header div div section .anchor {
  flex: 1 1;
}

.App-header div .account,
.App-header div div {
  display: flex;
  align-items: center;
}

.help {
  font-size: 1em;
  text-decoration: none;
  color: #6a6767;
  font-weight: 400;
  margin: 0 1em;
  white-space: nowrap;
}

.locations-container {
  display: flex;
  height: 200px;
  overflow-y: scroll;
  flex-wrap: wrap;
}

.locations-container div {
  display: flex;
  flex: 1 1 30%;
  flex-flow: wrap;
}

.location {
  border-left: 2px solid $location-color;
  padding: 14px;
  margin: 12px 0;
  flex: 1 1;
  min-width: 275px;
  max-width: 355px;
}

.location h1 {
  padding: 0;
  margin: 0;
  width: 296px;
  font-family: sans-serif;
  font-weight: 500;
  font-size: 20px;
  color: #454545;
  text-transform: uppercase;
}

.location span {
  font-family: "Roboto", sans-serif;
  font-size: 12px;
  color: #a3a3a3;
  width: 296px;
  font-weight: 400;
}

.location-secondary-info {
  display: flex;
  align-items: center;
}

.location-secondary-info span i {
  padding-right: 5px;
}

.location:hover {
  background-color: $location-color;
}

.location:hover h1 {
  color: $hover-color;
}

.location:hover span {
  color: $hover-color;
}

main {
  display: flex;
  flex-flow: column;
  flex: 1 1;
}

.pill {
  margin: 0 1em;
  border-radius: 100px;
  border: 1px solid $link-color;
  padding: 15px;
}

variables.scss:

$location-color: #49aaca;
$link-color: #333;
$hover-color: #fff;

The implementation inside of LocationList.js was the required task, the SCSS preprocessor is the extra credit. Using latest EcmaScript syntax is also extra credit, which I believe I have done. No duplicating of code is also extra credit, but I feel like my if conditional inside of LocationList component is duplicating code, but not sure how else that could have been done.

\$\endgroup\$
0
\$\begingroup\$

There’s no real harm in displaying an empty string when there is no airport code. Write the Jsx once and the line where you try to display the location.airport_code, just check if it exists and display it in that case only. With React, ternary operator is your friend:

I believe you can do this like:

renderLocation() {
    const filteredLocations = this.props.locations.filter(location => {
      return !location.name.match(/[A-Z0-9]+$/);
    });

    return filteredLocations.map(location => {
        return (
          <div key={location.id}>
            <div className="location">
              <h1>
                {location.name} {location.airport_code ? location.airport_code : ''}
              </h1>
              <div className="location-secondary-info">
                <span>
                  {location.airport_code ? 
                    <i className="material-icons">airplanemode_active</i> :
                    <i className="material-icons">location_city</i>
                  }  
                  {location.description}
                </span>
              </div>
            </div>
          </div>
        );

and remove the if statement entirely. That will remove your duplication. My only critique of the sass is that you didn’t really utilize any nesting which could have made your code a bit cleaner, but some people are against heavy nesting anyways. An example of utilizing nesting would be like

.location {
    &:hover {
      ....
    },
     h1 { 
      ...
    }
 }
\$\endgroup\$
  • \$\begingroup\$ Thank you for providing me with feedback. My concern in implementing the ternary expression would be, how would I handle the fact that there are two different icons in each condition? So with the location.airport_code I need one type of material-icons and with just the hotels, that is no location.airport_code I would need a different type of material-icons to be displayed. \$\endgroup\$ – Daniel Apr 11 at 16:56
  • \$\begingroup\$ @Daniel, same thing can be achieved with the ternary. I've updated my answer to show this \$\endgroup\$ – Jonathan Harvey Apr 11 at 18:50

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.