Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Urgent component #104

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Urgent component #104

wants to merge 10 commits into from

Conversation

leowang801
Copy link

Created a new question type "Urgent" where the total cost will be multiplied by a given multiplier (currently 1.5x) if the urgent question is selected "yes".

Screen.Recording.2023-03-31.at.2.06.27.AM.mov

Currently, there are some weird small issues with the costs that are displayed sometimes. Also, we may want to have an area where the user will be notified how much extra they will be charged for urgent requests.

@leowang801 leowang801 requested a review from Harin329 March 31, 2023 09:11
@@ -63,6 +63,7 @@ function SingleSelect({ question }) {
question: option,
},
});
console.log(option)
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this console log

@@ -3,10 +3,11 @@ import { useDispatch, useSelector } from "react-redux";
import { Navigate } from "react-router-dom";
import "./request-form.css";
import { appColor } from "../../constants";
import { LOAD_QUESTION } from "../../redux/actions/questionActions";
import { LOAD_QUESTION} from "../../redux/actions/questionActions";
Copy link
Member

Choose a reason for hiding this comment

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

lint: should have space after LOAD_QUESTION

@@ -109,7 +120,7 @@ function RequestForm() {
billableList.push({
service: service,
quantity: quantity,
cost: cost * quantity,
cost: isUrgent? cost * quantity * urgentMultiplier : cost * quantity,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cost: isUrgent? cost * quantity * urgentMultiplier : cost * quantity,
cost: isUrgent ? cost * quantity * urgentMultiplier : cost * quantity,

question: option,
},
});
console.log(option)
Copy link
Member

Choose a reason for hiding this comment

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

lets remove this log as well

question_type: 'urgent',
question: 'Is this request urgent?',
answer_id: uuid(),
answer: "No"});
Copy link
Member

Choose a reason for hiding this comment

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

We should probably use true or false here, but I think the answer is text typed, if that's the case, we should probably modify how we check the answer below.

return (
<div className="single-select-option" key={index}>
<FormControlLabel
value={option.answer}
Copy link
Member

Choose a reason for hiding this comment

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

this option.answer is likely a "yes" "No" string in your implementation, so it will always be true, maybe we should have it check whether it is "Yes" or "No"

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.

2 participants