Onboarding modal refactor #558
Conversation
|
[diff-counting] Significant lines: 148. |
|
Visit the preview URL for this PR (updated for commit d932744): https://cornelldti-courseplan-dev--pr558-onboarding-modal-ref-vlp3jype.web.app (expires Tue, 02 Nov 2021 23:59:06 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
zachary-kent
requested review from
benjamin-shen,
hahnbeelee and
willespencer
| required: false, | ||
| default: undefined, | ||
| }, | ||
| removeExam: { |
There was a problem hiding this comment.
Do all of these functions need to be passed down as properties to this new component? It might be cleaner to just $emit from this component and call the functions in OnboardingTransfer if the function definitions are going to remain in that file. I could see wanting to still pass down selectSubject and selectScore since there are different function definitions depending on exam type, but those could also be refactored to be 1 function that takes in an exam type parameter? What do you think?
There was a problem hiding this comment.
Yep that definitely makes sense! Shouldn't be too hard to change
Summary
This pull request refactors the transfer credit onboarding modal by factoring out the frontend code for AP and IB score input into one component. I'm going to add another input for CASE exams, so I thought it would be better to get this refactor out of the way now (instead of having to duplicate the same code used for AP/IB). No changes are visible to the client, and the modal looks the same as it did before.