Feedback

[Charlie-robin]

Feedback

Requirements:

1. A deployed website (using GitHub Pages): NOT DONE
2. A public GitHub repository for your codebase: DONE
3. Using Punk API: DONE
4. Tests where possible RTL: N/A

Plan

• Start:
  • Matching component tree & passing props? : STARTED
• Stage 1:
  • Search functionality? : STARTED
• Stage 2:
  • Filters x 3? : STARTED
• Stage 3:
  • Styled? : DONE

Score

MARKING-SCHEME

Category	Score
Functionality	30 / 55
Testing	0 / 10
React Use	15 / 20
Design	15 / 15
Total Score	60 / 100

Positive

• You have hooked your project up to an API!

• I feel confident with your understanding of components, props, and useState.

  • You have reusable presentational components that get given the data or the functions that they need. This is great :)
  • I think you have an idea of how func's as props work as you have implemented it as well.

• I can see that you understand useEffect but are not using the dependency array. This is because you don't need to based on your filtering.

  • When you have a chance read the docs and see if you can hook up your filter to use query params and the dependency array that works in junction with useEffect.

• Good going! it is a big project with lots of files When you get time use it as a playground for testing.

Constructive

• 60 is an amber which means you are on the fence, we need to help you get the project completed, I am going to give you some parts of the project to fix once they are done we can update the score.

• Make sure the NavBarProps handleSearchQuery prop matches the function type you are giving it.

type NavBarProps = {
  handleSearchQuery: (event: FormEvent<HTMLInputElement>) => void;
};

• Make sure you are giving the Navbar the searchQuery prop it needs.

<Navbar
  handleHighABVFilter={handleHighABVFilter}
  handleAcidicFilter={handleAcidicFilter}
  handleClassicRangeFilter={handleClassicRangeFilter}
  handleSearchQuery={handleSearchQuery}
  searchQuery={searchQuery}
/>

• Your CardList needs to be tidied up you have all the logic but it is messy, I have given you notes below.

const CardList = ({ highABV, acidic, classicRange, searchQuery }: CardListProps) => {
  const [text] = useState(""); // REMOVE THIS

  // SET THIS TO AN EMPTY ARRAY AS THE DEFAULT STATE
  // - THIS WILL STOP YOUR STATE BEING THIS UNION TYPE  Beer[] | undefined
  const [beers, setBeers] = useState<Beer[]>([]);

  const getBeers = async () => {
    // WHY IS THIS SET UP page=1?
    const url = `https://api.punkapi.com/v2/beers?page=1&per_page=80`;
    const res = await fetch(url);
    const data: Beer[] = await res.json();
    setBeers(data);
  };

  useEffect(() => {
    getBeers();
  }, []);

  // YOU CAN REMOVE THE beers? AS THE TYPE WILL NOT BE UNDEFINED NOW
  const filterBeers = (beers: Beer[]) => {
    // ONE LOOP WHERE YOU CHECK EACH OF YOUR CONDITIONS & THE SEARCH QUERY
    const filteredBeers = beers.filter(beer => {
      // EACH OF THESE RELATES TO EACH FILTER
      const hasSearchQuery = beer.name.toLowerCase().includes(searchQuery);
      // HERE THE CONDITION IS NEGATED SO IF THE highABV IS TRUE THE SECOND CONDITION WILL NEED TO BE TRUE
      // IF THE highABV IS FALSE THE CONDITION IS IGNORED
      const passesABVFilter = !highABV || beer.abv > 6;
      const passesAcidityFilter = !acidic || beer.ph < 4;
      // IF YOU HAVE MULTIPLE CONDITIONS YOU CAN USE VARIABLES AS LABELS TO MAKE ALL OF THE CONDITIONS EASIER TO UNDERSTAND
      // WITH THE YEAR RANGE YOU CAN GET THE YEAR AND COMPARE IT
      const firstBrewedYear = Number(beer.first_brewed.slice(3));
      const passesClassicFilter = !classicRange || firstBrewedYear < 2010;

      // THEN YOU CAN RETURN ALL OF THEM
      // THEY ALL NEED TO BE TRUE FOR THE BEER TO GET ADDED TO THE NEW ARRAY THAT WILL BE DISPLAYED
      return hasSearchQuery && passesABVFilter && passesClassicFilter && passesAcidityFilter;
    });

    return filteredBeers;
  };

  return (
    <div className="cardlist_container">
      {filterBeers(beers).map(beer => {
        return <Card beer={beer} />;
      })}
    </div>
  );
};

export default CardList;

• The project structure is untidy and not consistent can you organize it, please?
  • Let's keep it simple and just have individual folders for each of the components. Nesting them gets confusing and on a project this size it isn't necessary.
  • Your components folder should look like the example below.

- components/
  - NavBar/
    - NavBar.tsx
    - NavBar.scss

  - FilterList/
    - FilterList.tsx
    - FilterList.scss

  - FilterItem/
    - FilterItem.tsx
    - FilterItem.scss

  - SearchBox/
    - SearchBox.tsx
    - SearchBox.scss

  - Card/
    - Card.tsx
    - Card.scss

  - CardList/
    - CardList.tsx
    - CardList.scss

- data
    - beers.ts
    - types.ts

• We need to get this hosted so once the changes above are completed let's get it hosted ASAP.

---

[Charlie-robin]

Hey @fihockey thanks for making some of the fixes requested.

• The project is now more consistent and easier to navigate.
• The project is hosted and features are also working, good job.

I have updated the score to 70 which is in the green 👍.

Category	Score
Functionality	40 / 55
Testing	0 / 10
React Use	15 / 20
Design	15 / 15
Total Score	70 / 100