-
Notifications
You must be signed in to change notification settings - Fork 4
Class to functional components #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
pages/arts.jsx
Outdated
| <p> | ||
| Oops! We meant to redirect you to https://dailybruin.com/category/arts | ||
| </p>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure you return inside the if statement; otherwise, we're not doing anything with the
<p>text</p>tags
pages/arts.jsx
Outdated
| // Redirect | ||
| window.location.href = "https://dailybruin.com/category/arts-entertainment"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this in a useEffect, so that it only runs once (otherwise, this will run every time the component is re-rendered).
| Page.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it looks like we can get rid of slug (and thus context), as they are never used.
pages/editorial-board.jsx
Outdated
| Page.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can get rid of context and slug here
pages/category/spectrum.jsx
Outdated
| // const subcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${category[0].id}` | ||
| // ); | ||
| // const subcategories = [await subcategoriesRes.json()]; | ||
| // for (let i = 0; i < subcategories.length; i++) { | ||
| // const subsubcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${subcategories[i].id}` | ||
| // ); | ||
| // subcategories[i].subsubcategories = await subsubcategoriesRes.json(); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get rid of unecessary comments
pages/category/spectrum.jsx
Outdated
| const postsLinks = posts.map((category, index) => { | ||
| return ( | ||
| <> | ||
| <Head> | ||
| <title>{this.props.category[0].name + " - Daily Bruin"}</title> | ||
| </Head> | ||
| <div style={{ padding: "6px" }}> | ||
| <SectionHeader | ||
| category={this.props.category[0].name} | ||
| subcategories={sectionLinks} | ||
| /> | ||
| </div> | ||
| <MultimediaLayout | ||
| posts={this.props.posts} | ||
| categoryID={this.props.category[0].id} | ||
| <ul key={index}> | ||
| <li> | ||
| <Link | ||
| as={`/category/${category.slug}`} | ||
| href={`/category?slug=${category.slug}&apiRoute=category`} | ||
| > | ||
| <a>{category.title.rendered}</a> | ||
| </Link> | ||
| </li> | ||
| </ul> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like postsLinks is never used - can get rid of it.
pages/category/uncategorized.jsx
Outdated
| // Should take precedence over [slug].jsx | ||
| import PageWrapper from "../../layouts/PageWrapper"; | ||
| import React, { Component } from "react"; | ||
| import React from "react"; | ||
| import Error from "next/error"; | ||
|
|
||
| class Uncategorized extends Component { | ||
| render() { | ||
| return <Error statusCode={404} />; | ||
| } | ||
| function Uncategorized() { | ||
| return <Error statusCode={404} />; | ||
| } | ||
|
|
||
| export default PageWrapper(Uncategorized); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The more i look at it, I don't think this page is ever used. Can go ahead and get rid of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can remove Component from import
pages/category/breaking/index.jsx
Outdated
| ); | ||
| } | ||
|
|
||
| Tag.getInitialProps = async (context) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need context parameter
pages/category/cartoons.jsx
Outdated
| // const subcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${category[0].id}` | ||
| // ); | ||
| // const subcategories = await subcategoriesRes.json(); | ||
| // for (let i = 0; i < subcategories.length; i++) { | ||
| // const subsubcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${subcategories[i].id}` | ||
| // ); | ||
| // subcategories[i].subsubcategories = await subsubcategoriesRes.json(); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can get rid of unnecessary comments
pages/category/cartoons.jsx
Outdated
| Cartoons.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slug and context are not needed
pages/category/graphics.jsx
Outdated
| // const subcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${category[0].id}` | ||
| // ); | ||
| // const subcategories = await subcategoriesRes.json(); | ||
| // for (let i = 0; i < subcategories.length; i++) { | ||
| // const subsubcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${subcategories[i].id}` | ||
| // ); | ||
| // subcategories[i].subsubcategories = await subsubcategoriesRes.json(); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can get rid of unneeded comments
pages/category/graphics.jsx
Outdated
| Graphics.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slug and context unneeded
pages/category/illo.jsx
Outdated
| // const subcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${category[0].id}` | ||
| // ); | ||
| // const subcategories = await subcategoriesRes.json(); | ||
| // for (let i = 0; i < subcategories.length; i++) { | ||
| // const subsubcategoriesRes = await fetch( | ||
| // `${Config.apiUrl}/wp-json/wp/v2/categories?parent=${subcategories[i].id}` | ||
| // ); | ||
| // subcategories[i].subsubcategories = await subsubcategoriesRes.json(); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get rid of unneeded comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issues as arts page:
- return the p tags
- useEffect for redirect
pages/comment.jsx
Outdated
| Page.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need slug & context
pages/privacy.jsx
Outdated
| Page.getInitialProps = async (context) => { | ||
| const { slug } = context.query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unneeded slug & context. i'll stop writing this for other pages, but just remove unneeded slug & context for the rest of your changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- remove unnecessary imports
- showWelcome/setShowWelcome is unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This page doesn't even work, but it also didn't work previous to your change. I'll bring this up to my boss to see if it's even being used (maybe we can get rid of it — will get back to you on this)
|
@NarekGermirlian Can i get another review twin Im pretty sure I addressed everything you brought up |
NarekGermirlian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.