From 7ca3efe8b82803d9c3059c7fcf726792c7c91bdf Mon Sep 17 00:00:00 2001
From: Pujit Mehrotra <pujitm2009@gmail.com>
Date: Fri, 25 Oct 2024 10:46:58 -0400
Subject: [PATCH] doc(web): possibly ambiguous css & confusing cache
 policies/types

---
 web/components/Notifications/List.vue    | 23 +++++-----
 web/components/Notifications/Sidebar.vue |  9 +++-
 web/helpers/apollo-cache.ts              | 53 +++++++++++++++++++++++-
 3 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/web/components/Notifications/List.vue b/web/components/Notifications/List.vue
index 3ed46622d..decde3dcc 100644
--- a/web/components/Notifications/List.vue
+++ b/web/components/Notifications/List.vue
@@ -59,15 +59,16 @@ async function onLoadMore() {
 </script>
 
 <template>
-    <div
-      v-if="notifications?.length > 0"
-      v-infinite-scroll="onLoadMore"
-      class="divide-y divide-gray-200 overflow-y-auto pl-7 pr-4 h-full"
-    >
-      <NotificationsItem
-        v-for="notification in notifications"
-        :key="notification.id"
-        v-bind="notification"
-      />
-    </div>
+  <!-- The horizontal padding here adjusts for the scrollbar offset -->
+  <div
+    v-if="notifications?.length > 0"
+    v-infinite-scroll="onLoadMore"
+    class="divide-y divide-gray-200 overflow-y-auto pl-7 pr-4 h-full"
+  >
+    <NotificationsItem
+      v-for="notification in notifications"
+      :key="notification.id"
+      v-bind="notification"
+    />
+  </div>
 </template>
diff --git a/web/components/Notifications/Sidebar.vue b/web/components/Notifications/Sidebar.vue
index b6536a6dd..0132078b1 100644
--- a/web/components/Notifications/Sidebar.vue
+++ b/web/components/Notifications/Sidebar.vue
@@ -25,12 +25,19 @@ const { teleportTarget, determineTeleportTarget } = useTeleport();
       <BellIcon class="w-6 h-6" />
     </SheetTrigger>
 
-    <SheetContent :to="teleportTarget" class="w-full sm:max-w-[540px] h-screen px-0">
+    <!-- We remove the horizontal padding from the container to keep the NotificationList's scrollbar in the right place -->
+    <SheetContent
+      :to="teleportTarget"
+      class="w-full sm:max-w-[540px] h-screen px-0"
+    >
       <div class="flex flex-col h-full gap-3">
         <SheetHeader class="ml-1 px-6">
           <SheetTitle>Notifications</SheetTitle>
         </SheetHeader>
 
+        <!-- min-h-0 prevents the flex container from expanding beyond its containing bounds. -->
+        <!-- this is necessary because flex items have a default min-height: auto, -->
+        <!-- which means they won't shrink below the height of their content, even if you use flex-1 or other flex properties. -->
         <Tabs default-value="unread" class="flex-1 flex flex-col min-h-0">
           <div
             class="flex flex-row justify-between items-center flex-wrap gap-2 px-6"
diff --git a/web/helpers/apollo-cache.ts b/web/helpers/apollo-cache.ts
index bd67526a8..0363945fd 100644
--- a/web/helpers/apollo-cache.ts
+++ b/web/helpers/apollo-cache.ts
@@ -1,13 +1,54 @@
 import { InMemoryCache, type InMemoryCacheConfig } from "@apollo/client/core";
 
+/**------------------------------------------------------------------------
+ * !                    Understanding Cache Type Policies
+ *
+ *   The 'type' in type policies is a discriminated union of all graphql
+ * types recognized by Apollo (which defaults to discriminating on the
+ * __typename field).
+ *
+ * This means the depth or hiearchy of a field/type is irrelevant; the policy
+ * will always be [ParentType]: TypePolicy (where you can define the behavior
+ * of the field you actually care about).
+ *
+ * "Top-level" types are Query, Mutation, and Subscription. This is where your
+ * your top-level operations are defined. If you want to modify a sub-field or
+ * sub-operation, you need to look for its containing type.
+ *
+ * e.g. Query -> Notifications -> list
+ *
+ * The policy to modify `list`'s behavior will live in the policy for
+ * `Notifications`.
+ *------------------------------------------------------------------------**/
+
 const defaultCacheConfig: InMemoryCacheConfig = {
   typePolicies: {
     Notifications: {
       fields: {
         list: {
+          /**----------------------------------------------
+           * ?               Key Args Syntax
+           *
+           *  the sub-list in this context means "fields from the preceding key"
+           *
+           * i.e. this means [filter.type, filter.importance],
+           * not [filter, type, importance]
+           *---------------------------------------------**/
           keyArgs: ["filter", ["type", "importance"]],
+
+          /**
+           * Merges incoming data into the correct offset position. copied from
+           * [Apollo Docs](https://www.apollographql.com/docs/react/pagination/core-api#improving-the-merge-function).
+           *
+           * This lets paginated results live as a single list in the cache,
+           * which simplifies our client code.
+           *
+           * @param existing
+           * @param incoming
+           * @param context
+           * @returns the value to be cached
+           */
           merge(existing = [], incoming, { args }) {
-            // copied from https://www.apollographql.com/docs/react/pagination/core-api#improving-the-merge-function
             const offset = args?.filter?.offset ?? 0;
             const merged = existing.slice(0);
 
@@ -22,6 +63,16 @@ const defaultCacheConfig: InMemoryCacheConfig = {
     Mutation: {
       fields: {
         archiveAll: {
+          /**
+           * This clears the notifications cache when `archiveAll` is called to
+           * ensure that notification queries are invalidated, refetched,
+           * and in the correct state & sorting order after this operation.
+           *
+           * @param _ existing value in cache (irrelevant to this operation)
+           * @param incoming result from the server
+           * @param apolloContext contains the cache object
+           * @returns the value to cache for this operation
+           */
           merge(_, incoming, { cache }) {
             cache.evict({ fieldName: "notifications" });
             // Run garbage collection to clean up evicted references