-
Notifications
You must be signed in to change notification settings - Fork 0
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
Delete Lesson Functionality #89
base: master
Are you sure you want to change the base?
Changes from 55 commits
673b9f3
9526100
8d7c20b
b4168fe
c82cbeb
eb6fad2
9006d79
9ef2a55
93bb63a
bd62455
06679de
77d605f
e8ec859
6f0869d
e72bf32
1dee45c
19a1a33
ce13418
fbf6bbc
ecda341
27a06f3
1e1a8a3
b0cda0e
3c0062a
5d91020
d56ff8e
56fdef9
61d0c10
6f12d73
dccda21
ef866f3
8620cea
1e70ec8
0ac2c98
d7581bc
051fd05
e5b3e9f
5a577e0
f7b2507
6cb4bed
6ae462b
4dfb608
a12eb9f
93e7019
2ea9999
8151b73
dcc802f
42aa602
87ea524
ed96f65
f67052c
acb6571
c810c2e
a26f53c
6f42da7
799fca3
fac78b4
65d1a4c
fa679ce
97c1dda
24698be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import React from "react"; | ||
import "./DeleteConfirmation.scss"; | ||
|
||
function DeleteConfirmation(props) { | ||
return ( | ||
<div className="confirmation-background"> | ||
<div className="delete-note">Delete Lesson: {props.lessonName}?</div> | ||
<div className="delete-lesson-button-container"> | ||
<div className="yes-no-buttons" onClick={() => props.handleDelete()}> | ||
YES | ||
</div> | ||
<div | ||
className="yes-no-buttons" | ||
onClick={() => props.handleClose(false)} | ||
> | ||
NO | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
} | ||
|
||
export default DeleteConfirmation; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,44 @@ | ||
@import "../../pages/Index/index.scss"; | ||
|
||
.confirmation-background { | ||
background-color: white; | ||
border: 4px solid $assignments-font-color; | ||
box-shadow: "5px 10px #888888"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are the shadows all similar enough to standardize into a variable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this one is only used twice |
||
border-radius: 1vw; | ||
width: 30vw; | ||
min-height: 10vw; | ||
margin: auto; | ||
padding: 1vw; | ||
|
||
position: fixed; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why the random space |
||
left: 40%; | ||
top: 25%; | ||
} | ||
|
||
.delete-note{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. put a space here omg we need a formatter for css |
||
font-size: 1.5rem; | ||
color: black; | ||
font-weight: bold; | ||
} | ||
|
||
.delete-lesson-button-container { | ||
margin-top: 1vw; | ||
display:flex; | ||
flex-direction: row; | ||
} | ||
|
||
.yes-no-buttons { | ||
color: black; | ||
background-color: $assignments-light-purple; | ||
font-family: $assignments-roboto-font; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pls fix the indentation and rename to |
||
font-size: 1.5rem; | ||
width: 6vw; | ||
border-radius: 8px; | ||
transition-duration: 0.3s; | ||
font-weight: bold; | ||
margin: auto; | ||
cursor: pointer; | ||
&:hover { | ||
background-color: $assignments-light-green; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ function CustomLessonsDisplay(props) { | |
const [displayLessonInfo, setDisplayLessonInfo] = useState(false); | ||
const [createLessonRedirect, setCreateLessonRedirect] = useState(false); | ||
const [confirmationRedirect, setConfirmationRedirect] = useState(false); | ||
const [deletedLesson, setDeletedLesson] = useState(false); | ||
const [nameMap, setNameMap] = useState({}); | ||
const editLessonRedirect = props?.location.state?.redirect; | ||
|
||
|
@@ -32,7 +33,7 @@ function CustomLessonsDisplay(props) { | |
firebase.getAdminCustomLessons(ADMIN_ACCOUNT).then(lesson => { | ||
setAllLessons(lesson); | ||
}); | ||
}, [editLessonRedirect]); // Updates lessons when redirected to page from CreateAssignment | ||
}, [editLessonRedirect, deletedLesson]); // Updates lessons when redirected to page from CreateAssignment or lesson deleted | ||
|
||
async function deploymentNameMap(lesson) { | ||
let deploymentAccountIds = getDeploymentAccountIdsFromLesson(lesson); | ||
|
@@ -56,6 +57,10 @@ function CustomLessonsDisplay(props) { | |
setDisplayLessonInfo(display); | ||
} | ||
|
||
function handleDeletedLesson() { | ||
setDeletedLesson(!deletedLesson); | ||
} | ||
Comment on lines
+60
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was this the fix for the lessons not updating problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup! So this signals to the LessonsDisplay to recall the firebase endpoint and update the lessons displayed on the screen in useEffect() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you explain this? is the value of deletedlesson ignored and it's only used to trigger There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wdym the value of deletedLesson ignored? Yeah your interpretation is right, I made this specifically just to trigger useEffect(), to signal to the parent component displaying the customLessons (customLessonsDisplay) to update the lessons. Since we're not redirecting back to the same page, I think this is the best way to do it but correct me if I'm wrong if you have a better implementation! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just meant that it doesnt matter if it's true or false because its changing updates the component |
||
|
||
function handleClick(lesson) { | ||
handleChangeDisplayLessonInfo(!displayLessonInfo); | ||
setDisplayLesson(lesson); | ||
|
@@ -152,6 +157,7 @@ function CustomLessonsDisplay(props) { | |
lesson={displayLesson} | ||
template={displayLessonTemplate} | ||
setDisplay={handleChangeDisplayLessonInfo} | ||
handleDeletedLesson={handleDeletedLesson} | ||
nameMap={nameMap} | ||
/> | ||
)} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright bc this is frontend i'm assuming @AngelaLuo49021 wrote it correct me if i'm wrong
but to avoid clashing classnames while refactoring i decided to nest all the classes into this one because you can do that in sass, you can look at my branch (i think this is the link: #84 ) for examples