docs: add many inline comments

cornell-dti · Dec 3, 2024 · 31c151c · 31c151c
1 parent 67a9b1b
commit 31c151c
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 5 deletions.
diff --git a/src/components/Course/Course.vue b/src/components/Course/Course.vue
@@ -130,6 +130,8 @@ import trashGrayIcon from '@/assets/images/trash-gray.svg';
 import trashRedIcon from '@/assets/images/trash.svg';
 import Note from '../Notes/Note.vue';
 
+// MinimalNoteComponent is a representation of everything required for a functional,
+// but minimal Note component to work statefully.
 interface MinimalNoteComponent {
   note: string;
   isDirty: boolean;
@@ -188,6 +190,9 @@ export default defineComponent({
       trashIcon: trashGrayIcon, // Default icon
       courseCode: '',
       isExpanded: false,
+      // isNotVisible represents a small open 'portrusion' indicating that there is a note
+      // for the course in question. The note itself will not be visible without `isExpanded`
+      // being true as well.
       isNoteVisible: Boolean(this.courseObj.note),
       isShaking: false,
     };
@@ -290,6 +295,9 @@ export default defineComponent({
       if (!this.isNoteVisible) {
         this.isNoteVisible = true;
         this.menuOpen = false;
+        // NOTE: should use $nextTick, as the browser engine could optimize away the
+        // UI update by applying changes in the same render pass. Also important for allowing
+        // for time for the note component to be rendered before attempting to access it.
         this.$nextTick(() => {
           const noteComponent = this.$refs.note as MinimalNoteComponent | undefined;
           if (noteComponent) {
@@ -301,7 +309,7 @@ export default defineComponent({
         if (!noteComponent) {
           return;
         }
-        // Note already open — trigger a shake.
+        // Note already open — trigger a shake to indicate this to the user.
         if (noteComponent.isExpanded) {
           this.triggerCourseCardShake();
         } else {
@@ -324,10 +332,14 @@ export default defineComponent({
       }, 900); // 3 shakes * 0.3s = 0.9s
     },
     saveNote(note: string) {
+      if (!note || note === this.courseObj.note) {
+        return;
+      }
       this.$emit('save-note', this.courseObj.uniqueID, note);
     },
     handleClickOutsideNote(event: MouseEvent) {
-      // Don't count a click on the open note or three dots as a click outside.
+      // Don't count a click on the open note (.courseMenu) or three dots (.course-dotRow)
+      // as a click outside.
       const target = event.target as HTMLElement;
       if (target.closest('.courseMenu') || target.closest('.course-dotRow')) {
         return;
@@ -340,6 +352,7 @@ export default defineComponent({
       }
 
       if (noteComponent.isDirty) {
+        // Warn if the user is trying to leave a note with unsaved changes.
         this.triggerCourseCardShake();
       } else if (noteComponent.note && this.isNoteVisible) {
         noteComponent.collapseNote();
@@ -355,6 +368,8 @@ export default defineComponent({
     'click-outside': clickOutside,
   },
   watch: {
+    // NOTE: this is required for reactive deletion of notes client-side after the deletion
+    // modal is confirmed, as isNoteVisible is not reactive.
     'courseObj.note': {
       handler(newNote) {
         if (!newNote) {
@@ -375,6 +390,7 @@ export default defineComponent({
   padding-bottom: 20px;
 }
 
+// Emulates a slight side-to-side sway à la Figma micro-interaction.
 .figma-shake {
   animation: tilt-shaking 0.3s cubic-bezier(0.36, 0.07, 0.19, 0.97) 3;
   transform-origin: center center;

diff --git a/src/components/Notes/Note.vue b/src/components/Notes/Note.vue
@@ -1,6 +1,9 @@
 <template>
   <div class="note" :class="{ expanded: isExpanded }" :style="noteStyle" @click="handleClick">
     <div class="note-content" :class="{ visible: isExpanded, editing: isEditing }">
+      <!-- NOTE: the disabled attribute is used to prevent the user from being able to edit the note
+      when it is expanded, but the edit icon has not been pressed. This only applies when the note
+      is not empty (already had something written in it previously and read from the database). -->
       <input
         v-model="note"
         placeholder="Add a note..."
@@ -47,13 +50,16 @@ import { coursesColorSet } from '@/assets/constants/colors';
 export default defineComponent({
   name: 'Note',
   props: {
-    initialTranslateY: { type: String, required: true },
-    expandedTranslateY: { type: String, required: true },
+    initialTranslateY: { type: String, default: '-50px', required: true },
+    expandedTranslateY: { type: String, default: '-10px', required: true },
     width: { type: String, default: '200px' },
     color: { type: String, default: '#a8e6cf' },
     initialNote: { type: String, default: '' },
     lastUpdated: {
       type: [Object, Date],
+      // NOTE: we must use a Timestamp object here, as this is the internal type used by Firestore
+      // for storing the created JavaScript Date object. Helper functions are available by default
+      // on the Timestamp object to convert between the two.
       default: () => Timestamp.now(),
     },
   },
@@ -80,6 +86,7 @@ export default defineComponent({
         backgroundColor: this.getLighterColor(this.color),
       };
     },
+    // Displays
     formattedLastUpdated() {
       if (!this.lastUpdated) return '';
 

diff --git a/src/components/Semester/Semester.vue b/src/components/Semester/Semester.vue
@@ -481,6 +481,8 @@ export default defineComponent({
               ? {
                   ...course,
                   note,
+                  // We know that this must be an update as otherwise the frontend wouldn't allow
+                  // saveNote to be called.
                   lastUpdated: Timestamp.now(),
                 }
               : course
@@ -496,6 +498,8 @@ export default defineComponent({
         (semester: FirestoreSemester) => ({
           ...semester,
           courses: semester.courses.map(course =>
+            // NOTE: we must explicitly set note and lastUpdated to null, as Firestore cannot handle
+            // undefined values and extracting them would not be type-safe.
             course.uniqueID === uniqueID ? { ...course, note: null, lastUpdated: null } : course
           ),
         })

diff --git a/src/user-data.d.ts b/src/user-data.d.ts
@@ -19,7 +19,7 @@ type FirestoreSemesterCourse = {
   readonly semesters: readonly string[];
   readonly color: string;
   readonly note?: string | null;
-  readonly lastUpdated?: Timestamp | null;
+  readonly lastUpdated?: Timestamp | null; // NB: the Timestamp here is deliberately left untyped — importing from Firestore causes all sorts of namespace issues.
 };
 
 type FirestoreSemesterPlaceholder = {