Skip to content

Instantly share code, notes, and snippets.

@jxc876
Last active March 8, 2025 01:15
Show Gist options
  • Save jxc876/33e0b4e2fb91866346684f83e514f68f to your computer and use it in GitHub Desktop.
Save jxc876/33e0b4e2fb91866346684f83e514f68f to your computer and use it in GitHub Desktop.
Code Review

About

Code review for "Kili196/CV-Generator"

The project is built using vanilla React.js, HTML, & CSS + Vite

The App

The application is a resume builder.

Visually the Application looks like this

Good use of styling, nice to look at

image

image

The apps does not follow responsive design, however this is being worked on.

Components

Good use of components to break down structure:

<FormView .. />
<CvView .. />
// FormView
<SimpleInputField .. />
<SimpleInputField .. />
<SimpleInputField .. />
<InputBlock .. />
<TextArea .. />
// InputBlock
<AdvancedInputField .. />

image

These components are mostly as wrappers around <input> elements.

For advanced form validation Formik might be a good option

Improve Naming

Improve the name of components

"AdvancedInputField" doesn't convey much meaning.

  • Why is it "Advanced"? What is it doing?
  • It's hard to understand from reading the code

Visually it seems to render this

image

A better name might be InputFieldWithDateRange, or ResumeSection

State

State is stored at the top level:

function App() {
  const [user, setUser] = useState({
    firstname: "",
    lastname: "",
    email: "",
    phonenumber: "",
    address: "",
    aboutme: "",
    schools: [],
    works: [],
  });

image

And passed down to component to update or display

<FormView user={user} setUser={setUser} ... />
<CvView user={user} />

The leaf components then update the user state:

setUser()

Some of the name functions could be better named, ex: handleEventDoubleInput()

Despite reading it multiple times I don't understand the name.

Equality checks

User triple === over == to avoid type conversion

prev[type].find((element) => element.id == id)

Curly Braces

Curly braces are not needed when the values are simple strings

// NOT NEEDED
          <InputBlock
            heading={"School Journey:"}
            subheading={"School"}
            placeholder={"Enter school"}
            type={"schools"}
            handleEventDoubleInput={handleEventDoubleInput}
          />
// BETTER
          <InputBlock
            heading="School Journey:"
            subheading="School"
            placeholder="Enter school"
            type="schools"
            handleEventDoubleInput={handleEventDoubleInput}
          />
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment