Skip to content

add new view modal fields#460

Open
songmichael11 wants to merge 2 commits into
mainfrom
add-view-modal-fields
Open

add new view modal fields#460
songmichael11 wants to merge 2 commits into
mainfrom
add-view-modal-fields

Conversation

@songmichael11

Copy link
Copy Markdown
Collaborator

Description

Updated view review modal.

Motivation and Context

Closes #453

How has this been tested?

Looked at it.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ x] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Database migration
    • Ran pnpm db:generate and verified generated SQL migration files in packages/db/drizzle

Checklist:

  • [x ] My code follows the code style of this project.
  • [ x] I have moved the ticket to "In Review"
  • [ x] My change requires a change to the documentation.
  • [x ] I have updated the documentation accordingly.
  • [ x] I have added tests to cover my changes.
  • [ x] All new and existing tests passed.

@vercel

vercel Bot commented Jun 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cooper Ready Ready Preview, Comment Jun 23, 2026 1:24am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cooper-auth Skipped Skipped Jun 23, 2026 1:24am
cooper-docs Skipped Skipped Jun 23, 2026 1:24am

Request Review

@songmichael11 songmichael11 requested a review from gpalmer27 June 21, 2026 02:21

@gpalmer27 gpalmer27 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what i found while testing this:

  • the job length field on the form isn't required but it has a red asterisk next to it, we should make it required (and make sure all other fields with a star are required)
  • the interview rounds field is required but there's no star next to it, we should add that star

(sorry ik these aren't related to your changes lol but i feel like it goes well with this ticket)

Job length had a required asterisk but was optional; make it required.
Work hours and hourly pay were in the same state, so make them required
too. Hourly pay accepts "0" so the Unpaid toggle still satisfies it.
Add the missing asterisks to the required interview round Type and
Difficulty fields. Apply the same changes to the view/edit modal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gpalmer27

Copy link
Copy Markdown
Collaborator
Screenshot 2026-06-22 at 9 54 33 PM is this supposed to show the logo (i'm too lazy to check figma)

@gpalmer27

Copy link
Copy Markdown
Collaborator

There are some discrepancies between the field names in the review form vs. what's in the popup:

  • Year of co-op/internship term vs. Year
  • Job type vs. Employment type

Also other note: when filling out the review form, for the interview section, should it default to only having one round? i feel like it's a bit weird to default to having two required rounds, but also open to discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix view review modal

2 participants