Misc PR review edits.

Author: alserene

src/components/ContextFeedColumns.jsx

@@ -19,7 +19,7 @@ import FullScreenIcon from "../assets/FullScreenIcon.svg";
 
 const columnHelper = createColumnHelper();
 
-// TODO: Move to utils and import to here and dataLogColumns
+// TODO: Move to utils and import to here and dataLogColumns (OSW-1118)
 // Exact (multiple) match(es) filter function
 export const matchValueOrInList = (row, columnId, filterValue) => {
   const rowValue = row.getValue(columnId);
@@ -32,14 +32,13 @@ export const matchValueOrInList = (row, columnId, filterValue) => {
 };
 
 // Helper function to make time columns more readable
-// Luxon only natively supports milisecond precision, not microseconds.
+// Luxon only natively supports millisecond precision, not microseconds.
 // Will need to extra microseconds if this precision is required.
 function formatTimestamp(tsString) {
   if (!tsString) return null;
   const dt = DateTime.fromISO(tsString, { zone: "utc" });
   return dt.isValid ? dt.toFormat("yyyy-LL-dd HH:mm:ss.S") : tsString;
 }
-// }
 
 export const contextFeedColumns = [
   columnHelper.accessor("salIndex", {

src/pages/ContextFeed.jsx

@@ -1,13 +1,12 @@
 // Standard React hooks
-import { useEffect, useState } from "react";
+import { useEffect, useState, useMemo } from "react";
 
 // Libraries for the "toast" pop-up messages
 import { toast } from "sonner";
 import { Toaster } from "@/components/ui/sonner";
 
 // For routing / querying
 import { useNavigate, useSearch } from "@tanstack/react-router";
-// import { TELESCOPES } from "@/components/parameters";
 
 // UI components
 import { Skeleton } from "@/components/ui/skeleton";
@@ -31,7 +30,6 @@ import {
 } from "@/components/ContextFeedDefinitions.jsx";
 
 // Icons
-// import InfoIcon from "../assets/InfoIcon.svg";
 import DownloadIcon from "../assets/DownloadIcon.svg";
 
 // Utils
@@ -66,7 +64,7 @@ const filterDefaultEventsByTelescope = (telescope) => {
 // This is the page that gets rendered
 // as the Routing service's <Outlet /> in main.jsx
 function ContextFeed() {
-  // Get params from URL
+  // Subscribe component to URL params
   const { startDayobs, endDayobs, telescope, startTime, endTime } = useSearch({
     from: "/context-feed",
   });
@@ -77,11 +75,19 @@ function ContextFeed() {
   const queryEndDayobs = getDatetimeFromDayobsStr(endDayobs.toString())
     .plus({ days: 1 })
     .toFormat("yyyyMMdd");
-  // const instrument = TELESCOPES[telescope];
 
   // Time ranges for timeline
-  const [selectedTimeRange, setSelectedTimeRange] = useState([null, null]);
-  const [fullTimeRange, setFullTimeRange] = useState([null, null]);
+  // fullTimeRange doesn't need to be stored in state because it
+  // isn't modified by any child components, whereas selectedTimeRange
+  // is modified by the timeline.
+  const fullTimeRange = useMemo(
+    () => [
+      getDayobsStartUTC(String(startDayobs)),
+      getDayobsEndUTC(String(endDayobs)),
+    ],
+    [startDayobs, endDayobs],
+  );
+  const [selectedTimeRange, setSelectedTimeRange] = useState(fullTimeRange);
 
   // Storing and setting "state"
   // - [variable, setting function]
@@ -127,17 +133,12 @@ function ContextFeed() {
     });
   }, [telescope]);
 
-  // Set time ranges from url params.
+  // Set selectedTimeRange from url params.
   // Must be separate from general url-filter sync to avoid infinite renders.
   useEffect(() => {
     const startMillis = Number(startTime);
     const endMillis = Number(endTime);
 
-    // Compute full dayobs range
-    const startTimeRange = getDayobsStartUTC(String(startDayobs));
-    const endTimeRange = getDayobsEndUTC(String(endDayobs));
-    setFullTimeRange([startTimeRange, endTimeRange]);
-
     // Url params are optional; make sure we don't set NaNs
     const hasValidTimes =
       startMillis != null &&
@@ -152,9 +153,9 @@ function ContextFeed() {
       ]);
     } else {
       // Fallback to full dayobs range
-      setSelectedTimeRange([startTimeRange, endTimeRange]);
+      setSelectedTimeRange(fullTimeRange);
     }
-  }, [startTime, endTime, startDayobs, endDayobs]);
+  }, [startTime, endTime, fullTimeRange]);
 
   // Sync url time params with selectedTimeRange
   useEffect(() => {
@@ -169,6 +170,7 @@ function ContextFeed() {
     const newEnd = selectedTimeRange[1].toMillis();
 
     if (currentStart !== newStart || currentEnd !== newEnd) {
+      // Mutate browser history with new times
       navigate({
         to: "/context-feed",
         search: {
@@ -268,7 +270,6 @@ function ContextFeed() {
 
     // Fetch the Context Feed data
     fetchContextFeed(startDayobs, endDayobs, abortController)
-      // Just collecting columns for now, until dataframe bug is sorted
       .then(([data]) => {
         // Prepare data for timeline and table
         let currentTask = null;
@@ -281,7 +282,7 @@ function ContextFeed() {
             let eventColor = salInfo.color ?? "#ffffff";
 
             // Split Narrative Log into variants
-            // Narrative Log splitting
+            // TODO: Adapt when this comes from rubin_nights (OSW-1119)
             if (salInfo.label === "Narrative Log") {
               if (NARRATIVE_LOG_MAP.AuxTel.includes(entry.name)) {
                 salInfo = SAL_INDEX_INFO[7];
@@ -296,7 +297,7 @@ function ContextFeed() {
             }
 
             if (
-              // TODO: swap to "Task Change" once changed in rubin_nights
+              // TODO: swap to "Task Change" (OSW-1119)
               entry.finalStatus === "Job Change"
             ) {
               currentTask = entry.name;
@@ -315,8 +316,7 @@ function ContextFeed() {
           // Chronological order
           .sort((a, b) => a.event_time_millis - b.event_time_millis);
 
-        // Set the fetched cols to state.
-        // setContextFeedCols(cols);
+        // Set the fetched data to state.
         setContextFeedData(preparedData);
 
         // Warn if no data returned but no error
@@ -438,6 +438,7 @@ function ContextFeed() {
                     {Object.entries(SAL_INDEX_INFO)
                       .filter(([key]) => key !== "10" && key !== "0") // exclude AUTOLOG & NL
                       .sort(
+                        // sort on displayIndex
                         ([, aInfo], [, bInfo]) =>
                           (aInfo.displayIndex ?? 0) - (bInfo.displayIndex ?? 0),
                       )