25 + 5 Clock Project Feedback

Hello FreeCodeCampers!

I was wondering if I could get some feedback on my 25 + 5 clock. I enjoyed this project a lot. I felt like it really made me learn a lot more about react.

here is a link to the project:
Vercel: https://fcc-25-5-timer-codyjamesbrooks.vercel.app/

And here is my repo for it, if you wanna look at any code I didn’t include in this post.
Github: GitHub - codyjamesbrooks/FCC-25-5-timer: Break/Work timer using React

I copied the App.js code below so that you don’t have to jump through hoops to look under the hood.

Generally I feel like it is helpful to add specific thing that I did/questions I had in order to kick start any comments, so here are a couple of those

  1. All of my components are built as class components. My questions is about functional components. Basically what I want to know is why? What are the disadvantages to just building everything into classes? Why are hooks all the rage? What is a situation where a function component would really shine over just building a class?

  2. The last project that I posted for feedback, I get some an excellent comments from @lasjorg regarding paying a bit more attention to my function names. I tried to keep that in mind and I am curious to know if they are clear.

  3. I compute displayMinutes, and displaySeconds inside the render method. Is that ok?
    I am also declaring those variables in that render method. Now correct me if I am wrong, but does that mean I am redeclaring AND computing those variables every time render is called? Am I understanding that correctly, and is that best-practice?

  4. Lastly how are the comments? Enough? To much? Feel free to comment on anything that I didn’t mention.

Thanks for any feedback provided. Thanks for taking the time. Thanks for being such a great community.

import React from "react";
import "./App.css";

import BreakSessionSet from "./BreakSessionSet";
import TimerControls from "./TimerControls";
import Countdown from "./Countdown";

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      break: 5,
      session: 25,
      displayTime: 1500,
      label: "Session",
      timerRunningFlag: false,
    };
    // functions for adjusting break and session length
    this.handleBreakSet = this.handleBreakSet.bind(this);
    this.handleSessionSet = this.handleSessionSet.bind(this);

    // functions to manage timer countdown
    this.decreaseDisplayTime = this.decreaseDisplayTime.bind(this);

    // functions to handle timer control buttons.
    this.handleStartClick = this.handleStartClick.bind(this);
    this.handleStopClick = this.handleStopClick.bind(this);
    this.handleResetClick = this.handleResetClick.bind(this);
  }
  switchBetweenSessionAndBreak() {
    // when the display timer reaches zero this function is called and manages the transation
    // from a session to a break. Changes the timer label, Resets the display time
    if (this.state.label === "Session") {
      this.setState((state) => ({
        displayTime: state.break * 60,
        label: "Break",
      }));
    } else {
      this.setState((state) => ({
        displayTime: state.session * 60,
        label: "Session",
      }));
    }
  }
  decreaseDisplayTime() {
    // Decreases the displayTime by 1. When the display time hits zero, an audio element is
    // played and the function switchBetweenSessionBreak is called.
    if (this.state.displayTime !== 0) {
      this.setState((state) => ({
        displayTime: --state.displayTime,
      }));
    } else {
      let sound = document.getElementById("beep");
      sound.play();
      this.switchBetweenSessionAndBreak();
    }
  }
  handleStartClick() {
    // Create an interval element that calls decreaseDisplayTime every 1000ms
    // then updates the timerRunningFlag to indicated that the timer is running
    this.countDown = setInterval(this.decreaseDisplayTime, 1000);
    this.setState({ timerRunningFlag: true });
  }
  handleStopClick() {
    // Destroys the interval element stopping DisplayTime from decreasing.
    // then updates the timerRunningFlag to indicated that the timer is stopped
    clearInterval(this.countDown);
    this.setState({ timerRunningFlag: false });
  }

  handleBreakSet(number) {
    // Function handles the break set arrows. Adjust the state variable break
    // by +1 or -1. Conditions added in to ensure that the 1 <= break <= 60.
    // also prevents the break length from being adjust while the timer is running.
    if (
      (this.state.break > 1 || number > 0) &&
      this.state.timerRunningFlag === false
    ) {
      this.setState((state) => ({
        break: Math.min(60, state.break + number),
      }));
    }
  }
  handleSessionSet(number) {
    // Function behaves the same as 'handleBreakSet' only acts on the session
    // state variable. Also updates the displayTime to reflect changes in session length
    if (
      this.state.session + number >= 1 &&
      this.state.session + number <= 60 &&
      this.state.timerRunningFlag === false
    ) {
      this.setState((state) => ({
        session: state.session + number,
        displayTime: state.displayTime + number * 60,
      }));
    }
  }

  handleResetClick() {
    // When reset is clicked any countdown timer will be stopped, The audio element will
    // be reloaded, and state variables will be returned to their starting values.
    if (this.countDown) {
      clearInterval(this.countDown);
    }
    let sound = document.getElementById("beep");
    sound.load();
    this.setState({
      break: 5,
      session: 25,
      displayTime: 1500,
      label: "Session",
      timerRunningFlag: false,
    });
  }

  render() {
    // When render is called we calculate two values displayMinutes, and displaySeconds.
    // Both values are restricted to display two digits.
    let displayMinutes = Math.floor(
      this.state.displayTime / 60
    ).toLocaleString("en-US", { minimumIntegerDigits: 2 });
    let displaySeconds = (this.state.displayTime % 60).toLocaleString("en-US", {
      minimumIntegerDigits: 2,
    });
    // Then Return the app components.
    return (
      <div id="app-container">
        <div id="title">
          <h1 id="app-name">25 + 5 Clock</h1>
          <h6 id="author">coded by cody</h6>
        </div>
        <BreakSessionSet
          break={this.state.break}
          session={this.state.session}
          handleBreakSet={this.handleBreakSet}
          handleSessionSet={this.handleSessionSet}
        />
        <Countdown
          label={this.state.label}
          displayMinutes={displayMinutes}
          displaySeconds={displaySeconds}
        />
        <TimerControls
          timerRunningFlag={this.state.timerRunningFlag}
          handleResetClick={this.handleResetClick}
          handleStartClick={this.handleStartClick}
          handleStopClick={this.handleStopClick}
        />
        <audio
          id="beep"
          src="https://actions.google.com/sounds/v1/alarms/bugle_tune.ogg"
        />
      </div>
    );
  }
}
export default App;

1 Like

@codyjamesbrooks Hi, Nice project. I dig the color combination and the fact that the timer works is really good. Here are a few things that I noticed about the code. This is strictly my opinion on how you can make the project better.

  • Fix the mobile view by:
    -increasing the size of the h1 .app-name element
    -Make the divs inside set-timers-container fit on the screen.

  • Use rem and em instead of px.

  • Use a monospace font for the clock digits because monospace fonts have the same width. You can see the numbers visibly shifting once the clock gets down to 1. The number one is more narrow than the other numbers.

  • Add a fallback font like this: font-family: 'Londrina Solid', monospace;.

  • Add a sound effect when break time starts and ends.

  • You can change the color during the break time as a visual notification that break time has started.

I like the comments so that beginners can understand the code better and learn from it. The good thing is that the comments do not affect the way the code works.

thanks @brandon_wallace. I honestly totally forgot about mobile… Which was a huge oversight, that I really should have considered.

You also pointed out two bad habits that I have: which is using px, and forgetting fallback fonts.

I am also realizing that I could have cut out more of the boilerplate that comes with create-react-app.

Thanks for having a look at it though!