790 allow plans with no concentration #803
Conversation
…hing appear saying that you have an undecided major
…ratio and a more nonchalant red italicized aesthetic subtitle informing you the same
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -287,6 +287,7 @@ export function validateMajor2( | |||
| ): MajorValidationResult { | |||
| const tracker = new Major2ValidationTracker(taken); | |||
| let concentrationReq: Requirement2[] = []; | |||
| console.log(concentrations); // this is "undecided" | |||
| return false; | ||
| } | ||
|
|
||
| console.log("ASDASDSA"); |
There was a problem hiding this comment.
just updated 👍 thanks wennis
| {subtitle} | ||
| </Text> | ||
| )} | ||
| </Box> | ||
| <ConcentrationDropdownWarning /> |
There was a problem hiding this comment.
should this be in a conditional?
| ); | ||
| const isValidConcentrationName = | ||
| concentrations.includes(concentrationName) || | ||
| concentrationName.toLowerCase() === "undecided"; |
There was a problem hiding this comment.
we should standardize how we are comparing to "undecided" vs "Undecided" string
packages/common/src/utils.ts
Outdated
| ): OptionObject[] => { | ||
| const optionObjects = convertToOptionObjects(options); | ||
| optionObjects.unshift({ | ||
| label: "Undecided", |
There was a problem hiding this comment.
we can create a UNDECIDED_OPTION constant
There was a problem hiding this comment.
Sounds good, updated in latest push
…nvertToOptionObjects method
| ); | ||
| const isValidConcentrationName = | ||
| concentrations.includes(concentrationName) || | ||
| concentrationName === "Undecided"; |
There was a problem hiding this comment.
put "Undecided" as a constant
| : "primary.blue.dark.main" | ||
| } | ||
| fontStyle={ | ||
| subtitle === "Concentration Undecided" ? "italic" : "normal" |
There was a problem hiding this comment.
also can be a constant value
| @@ -36,14 +41,20 @@ export const majorOptionObjectComparator = ( | |||
| /** | |||
| * Converts a list of strings or numbers into a list of option objects for the | |||
| * Select component. | |||
| * | |||
| @@ -319,7 +319,8 @@ export const AddPlanModal: React.FC<AddPlanModalProps> = ({ | |||
| name="concentration" | |||
| placeholder="Select a Concentration" | |||
| options={convertToOptionObjects( | |||
| majorConcentrations.concentrations | |||
| majorConcentrations.concentrations, | |||
| true | |||
There was a problem hiding this comment.
Do you ever pass in false? Both times when you use convertToOptionObjects its true
| ); | ||
| const isValidConcentrationName = | ||
| concentrations.includes(concentrationName) || | ||
| concentrationName === UNDECIDED; |
There was a problem hiding this comment.
I think we can change constant names to like UNDECIDED_STRING or UNDECIDED_STR for example to be a little more specific. Otherwise looks good
Description
For all majors that have concentrations, you can now select undecided as an option.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Frontend changes to add undecided as an option when selecting concentrations as well as displaying warning dropdowns letting you know that the concentration is undecided and will eventually need changing. Also some backend validation changes that allow an undecided concentration to exist even though it's not defined anywhere in the major.
Closes #790
Type of change
Please tick the boxes that best match your changes.
yarn installyarn dev:migration:runHow Has This Been Tested?
Visually