#2136 finalize design review modal

[superhvarn]

Changes

Aaryan -
created a modal and linked it to the user availability page via finalize button
I also changed the look for the meeting type cuz i thought it looked cooler, lmk if I should change it to what it showed in the mock

Screenshots

Checklist

It can be helpful to check the Checks and Files changed tabs.
Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.

• All commits are tagged with the ticket number
• No linting errors / newline at end of file warnings
• All code follows repository-configured prettier formatting
• No merge conflicts
• All checks passing
• Screenshots of UI changes (see Screenshots section)
• Remove any non-applicable sections of this template
• Assign the PR to yourself
• No yarn.lock changes (unless dependencies have changed)
• Request reviewers & ping on Slack
• PR is linked to the ticket (fill in the closes line below)

Closes # (issue #2136 )

[Aaryan1203]

requested some changes

[RChandler234]

ok so, I've realized we need a way to actually see what the set time is/ set a new time on the design review detail page. Can you add that (I think we can do name and a day/ time range selector on one row and then required and optional attendee selectors on the row below)

We should also be displaying that info (when the design review is actually scheduled for) at the top of the modal

[walker-sean]

Love how this is coming together!

Quick review, will look through code later:

• We need to have a field for zoom link (preferably it is disabled, is cleared, and does not submit when the meeting is not online - and same thing with location/in person)
• The time selected must be on an hour - we could use a validator but it would be nicer to not let users select a time that doesn't fall on an hour.
• Display the name and when the dr is scheduled at the top of the modal

[walker-sean]

Looking really good!

[RChandler234]

Several Comments:

• None of the useEffect, useMemo stuff is necessary. While they are tools that React has, you should use them very sparingly because they cause re-renders (or limit them in useMemos case). For useMemo, I would almost never use it (see more details below). For useEffect, in our case we store all our state in the backend, so in most cases it makes the most sense to refetch new state from the backend using a query invalidation and cause re-renders using useState or props (whenever either changes it causes the re-render of a page).
• You should try whenever possible to localize super weird/ intensive data reorganizing. For instance you'll see I moved a lot of the existingMeetingData/ conflictingMeeting stuff down a component or two. This 1, helps clear up the main file you were working, and 2, helps reduce the load on re-rendering because the component has fewer sub-component that it will need to rerender, and 3 can help reduce the number of props you're passing around if you can move multiple calculations
• When you're passing around data, it's usually better to do name transformation only when you need to. You can see the way I passed around the selectedDate or designReview rather than the specific name because there's no need to limit what we're passing to just the designReview name, especially when we're going through multiple layers of components.
• I think the main failing of this PR was that things got way too complicated. You should whenever possible be making things as simple as possible (especially with big wire-ups like this) because, at least in my book, simple = better (easier for other people to use, less prone to error, shorter code). Ways to mitigate this are:
• 1. trying to make sure components are properly abstracted (a component should have one purpose and anything not related to that should tried to be moved to a sub component or a helper method where possible)
• 2. I kinda like to say "don't do too much" (for example those time pickers are cool, but for our purposes are a bit complicated and give the user access to options they can't actually choose)
• 3. If you feel yourself getting to a point where you're like "this file is getting above 200 lines" or "wow this is super complicated," either take a bit and come back to the code or ask someone to take a look because (expect in rare cases) you're probably overcomplicating something

tl;dr I think you did really well on this PR, all of the logic was correct, I literally just reshuffled things to make the code nicer and it mostly works fine. The main takeaway is "Keep it Simple Stupid"

[RChandler234]

I'm still fixing up some the design/ the finalize functionality but this code should look a lot nicer

[RChandler234]

LGTM 🔥