Add delete events feature #18
Conversation
src/App.js
Outdated
| jsonObj.forEach((delEvent) => { | ||
| console.log(startTime, delEvent.title); | ||
|
|
||
| if ( | ||
| delEvent.title === eventName | ||
| && delEvent.start === startTime | ||
| && delEvent.end === endTime | ||
| ) { | ||
| return; | ||
| } | ||
| newCustomList.push(delEvent); | ||
| }); |
There was a problem hiding this comment.
This could be replaced by a filter method, but it probably won't impact performance anyways
| return; | ||
| } | ||
| newCustomList.push(delEvent); | ||
| }); |
There was a problem hiding this comment.
Get rid of the unnecessary console logs. Variables like jsonObj need to be changed to more meaningful names.
There was a problem hiding this comment.
I agree. Variable names like delEvent can be improved. Especially because delEvent is actually the event which stays and is not being deleted!
A generic name for the function argument like event might suffice.
|
oooo yea i forgot to beautify this my bad. Will do.
…On Mon, Apr 11, 2022 at 1:14 PM butter-chicken27 ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In src/App.js
<#18 (comment)>
:
> + const removeEvent = (eventName, startTime, endTime) => {
+ const jsonObj = JSON.parse(localStorage.getItem(customKey)) || [];
+
+ const newCustomList = [];
+ jsonObj.forEach((delEvent) => {
+ console.log(startTime, delEvent.title);
+
+ if (
+ delEvent.title === eventName
+ && delEvent.start === startTime
+ && delEvent.end === endTime
+ ) {
+ return;
+ }
+ newCustomList.push(delEvent);
+ });
Get rid of the unnecessary console logs. Variables like jsonObj need to be
changed to more meaningful names.
------------------------------
In src/pages/TimeTable.js
<#18 (comment)>
:
> @@ -30,6 +31,7 @@ function TimeTable({ eventList, handleNewCustomEvent }) {
let eventDate = null;
let startTime = null;
let endTime = null;
+ // let note = "";
Delete the comment
------------------------------
In src/pages/TimeTable.js
<#18 (comment)>
:
> @@ -214,6 +235,13 @@ function TimeTable({ eventList, handleNewCustomEvent }) {
shrink: true,
}}
/>
+ {/* <TextField
+ margin="dense"
+ id="notes"
+ label="Add note"
+ fullWidth
+ onChange={handleNoteChange}
+ /> */}
*Another* comment to be deleted
—
Reply to this email directly, view it on GitHub
<#18 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AUL67E7ZUGAAVNXUGH2HVZLVEPJ6DANCNFSM5S6VEHBQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Disclaimer:- This footer text is to convey that this email is sent by one
of the users of IITH. So, do not mark it as SPAM.
|
- Windows error popup when trying to delete AIMS event - Changed variable names - Removed console logs and unnecessary comments
There was a problem hiding this comment.
It seems that the eslint and prettier hooks were not run, again.
There is also an eslint issue (react/forbid-prop-types) causing some issues atm.
Also, the AIMS Error mesages sdoes not show up if you're using the newer extension, in which the title also contains the course names.
src/pages/TimeTable.js
Outdated
| clickInfo.event.remove(); | ||
| handleDeleteEvent( | ||
|
|
||
| const aimsTitle = aimsCourse.identifiedCourses.find(course => course === clickInfo.event.title) |
There was a problem hiding this comment.
This is not going to work in the newer version of the extension, where the course title also includes the course name.
src/pages/TimeTable.js
Outdated
| if (aimsTitle !== undefined){ | ||
| window.alert('AIMS courses cannot be deleted.'); | ||
| } | ||
| else { |
|
The latest pushed commit is not related to this PR. Please make a new pull request with the corresponding topic, and with only the latest commit and not the earlier commit. Thanks! |
No description provided.