fix: improve click behaviour of single note

* fix: improve click behaviour of single note

* fix: protect from unexpected undefined

Author: chmurson

src/components/LocationProvider/LocationProvider.tsx

@@ -2,30 +2,27 @@ import { type ISelectionParams, type ISynctexBlock, isSameBlock } from "@fluffyl
 import { type ReactNode, createContext, useCallback, useContext, useEffect, useMemo, useState } from "react";
 import { deserializeLegacyLocation } from "../../utils/deserializeLegacyLocation";
 import { type IMetadataContext, MetadataContext } from "../MetadataProvider/MetadataProvider";
+import type { ILocationParams, SearchParams } from "./types";
+import {
+  BASE64_VALIDATION_REGEX,
+  SEGMENT_SEPARATOR,
+  SELECTION_DECOMPOSE_PATTERN,
+  SELECTION_SEGMENT_INDEX,
+  VERSION_SEGMENT_INDEX,
+} from "./utils/constants";
+import { locationParamsToHash } from "./utils/locationParamsToHash";
 
 export interface ILocationContext {
   locationParams: ILocationParams;
   setLocationParams: (newParams: ILocationParams) => void;
   synctexBlocksToSelectionParams: (blocks: ISynctexBlock[]) => ISelectionParams;
-}
-
-interface ILocationParams extends Partial<ISelectionParams> {
-  version: string;
-  search?: string;
-  section?: string;
+  getHashFromLocationParams: (params: ILocationParams) => string;
 }
 
 interface ILocationProviderProps {
   children: ReactNode;
 }
 
-const VERSION_SEGMENT_INDEX = 0;
-const SELECTION_SEGMENT_INDEX = 1;
-const SEGMENT_SEPARATOR = "/";
-const SELECTION_DECOMPOSE_PATTERN = /[0-9A-F]{6}/gi;
-const SHORT_COMMIT_HASH_LENGTH = 7; // as many as git uses for `git rev-parse --short`
-const BASE64_VALIDATION_REGEX = /^#[-A-Za-z0-9+/]*={0,3}$/;
-
 export const LocationContext = createContext<ILocationContext | null>(null);
 
 export const useLocationContext = () => {
@@ -53,30 +50,17 @@ export function LocationProvider({ children }: ILocationProviderProps) {
 
   const handleSetLocationParams = useCallback(
     (newParams?: ILocationParams) => {
-      const fullVersion = newParams?.version;
-      const version = fullVersion
-        ? fullVersion.substring(0, SHORT_COMMIT_HASH_LENGTH)
-        : metadata.versions[metadata.latest]?.hash.substring(0, SHORT_COMMIT_HASH_LENGTH);
-      const versionName = fullVersion ? metadata.versions[fullVersion]?.name : metadata.versions[metadata.latest]?.name;
-
-      const stringifiedParams = [];
-
-      stringifiedParams[VERSION_SEGMENT_INDEX] = version;
-
-      if (newParams?.selectionStart && newParams?.selectionEnd) {
-        stringifiedParams[SELECTION_SEGMENT_INDEX] = [
-          encodePageNumberAndIndex(newParams.selectionStart.pageNumber, newParams.selectionStart.index),
-          encodePageNumberAndIndex(newParams.selectionEnd.pageNumber, newParams.selectionEnd.index),
-        ].join("");
-      }
+      if (!newParams) return;
+      const hash = locationParamsToHash(newParams, metadata);
+      window.location.hash = hash;
+    },
+    [metadata],
+  );
 
-      // we never put search/section to the URL,
-      // yet we keep them in `locationParams`.
-      const params: SearchParams = {
-        v: versionName,
-        rest: `${SEGMENT_SEPARATOR}${stringifiedParams.join(SEGMENT_SEPARATOR)}`,
-      };
-      window.location.hash = serializeSearchParams(params);
+  const getHashFromLocationParams = useCallback(
+    (params: ILocationParams) => {
+      const hash = locationParamsToHash(params, metadata);
+      return hash;
     },
     [metadata],
   );
@@ -191,8 +175,9 @@ export function LocationProvider({ children }: ILocationProviderProps) {
       locationParams,
       setLocationParams: handleSetLocationParams,
       synctexBlocksToSelectionParams,
+      getHashFromLocationParams,
     };
-  }, [locationParams, handleSetLocationParams, synctexBlocksToSelectionParams]);
+  }, [locationParams, handleSetLocationParams, synctexBlocksToSelectionParams, getHashFromLocationParams]);
 
   if (!context) {
     return null;
@@ -201,11 +186,6 @@ export function LocationProvider({ children }: ILocationProviderProps) {
   return <LocationContext.Provider value={context}>{children}</LocationContext.Provider>;
 }
 
-function encodePageNumberAndIndex(pageNumber: number, index: number) {
-  const asHexByte = (num: number) => (num & 0xff).toString(16).padStart(2, "0");
-  return `${asHexByte(pageNumber)}${asHexByte(index)}${asHexByte(index >> 8)}`;
-}
-
 function decodePageNumberAndIndex(s: string) {
   if (s.length > 6) throw new Error("Pass exactly 6 hex characters");
   const fromHex = (s: string) => Number(`0x${s}`);
@@ -215,13 +195,6 @@ function decodePageNumberAndIndex(s: string) {
   return { pageNumber, index };
 }
 
-type SearchParams = {
-  rest: string;
-  v?: string;
-  search?: string;
-  section?: string;
-};
-
 function extractSearchParams(hash: string): SearchParams {
   // skip the leading '/'
   const [rest, searchParams] = hash.substring(1).split("?");
@@ -246,13 +219,3 @@ function extractSearchParams(hash: string): SearchParams {
 
   return result;
 }
-function serializeSearchParams({ rest, ...searchParams }: SearchParams) {
-  const search = [];
-  for (const key of Object.keys(searchParams)) {
-    const val = searchParams[key as keyof typeof searchParams];
-    if (val) {
-      search.push(`${key}=${encodeURIComponent(val)}`);
-    }
-  }
-  return `${rest}${search.length > 0 ? `?${search.join("&")}` : ""}`;
-}

src/components/LocationProvider/types.ts

@@ -0,0 +1,14 @@
+import type { ISelectionParams } from "@fluffylabs/links-metadata";
+
+export interface ILocationParams extends Partial<ISelectionParams> {
+  version: string;
+  search?: string;
+  section?: string;
+}
+
+export type SearchParams = {
+  rest: string;
+  v?: string;
+  search?: string;
+  section?: string;
+};

src/components/LocationProvider/utils/constants.ts

@@ -0,0 +1,6 @@
+export const VERSION_SEGMENT_INDEX = 0;
+export const SELECTION_SEGMENT_INDEX = 1;
+export const SEGMENT_SEPARATOR = "/";
+export const SELECTION_DECOMPOSE_PATTERN = /[0-9A-F]{6}/gi;
+export const SHORT_COMMIT_HASH_LENGTH = 7; // as many as git uses for `git rev-parse --short`
+export const BASE64_VALIDATION_REGEX = /^#[-A-Za-z0-9+/]*={0,3}$/;

src/components/LocationProvider/utils/encodePageNumberAndIndex.ts

@@ -0,0 +1,4 @@
+export function encodePageNumberAndIndex(pageNumber: number, index: number) {
+  const asHexByte = (num: number) => (num & 0xff).toString(16).padStart(2, "0");
+  return `${asHexByte(pageNumber)}${asHexByte(index)}${asHexByte(index >> 8)}`;
+}

src/components/LocationProvider/utils/locationParamsToHash.ts

@@ -0,0 +1,49 @@
+import type { IMetadataContext } from "../../MetadataProvider/MetadataProvider";
+import type { ILocationParams, SearchParams } from "../types";
+import {
+  SEGMENT_SEPARATOR,
+  SELECTION_SEGMENT_INDEX,
+  SHORT_COMMIT_HASH_LENGTH,
+  VERSION_SEGMENT_INDEX,
+} from "./constants";
+import { encodePageNumberAndIndex } from "./encodePageNumberAndIndex";
+
+export const locationParamsToHash = (params: ILocationParams, metadata: IMetadataContext["metadata"]) => {
+  const fullVersion = params.version;
+  const version = fullVersion
+    ? fullVersion.substring(0, SHORT_COMMIT_HASH_LENGTH)
+    : metadata.versions[metadata.latest]?.hash.substring(0, SHORT_COMMIT_HASH_LENGTH) ?? "";
+  const versionName =
+    (fullVersion ? metadata.versions[fullVersion]?.name : metadata.versions[metadata.latest]?.name) ?? "";
+
+  const stringifiedParams = [];
+
+  stringifiedParams[VERSION_SEGMENT_INDEX] = version;
+
+  if (params.selectionStart && params.selectionEnd) {
+    stringifiedParams[SELECTION_SEGMENT_INDEX] = [
+      encodePageNumberAndIndex(params.selectionStart.pageNumber, params.selectionStart.index),
+      encodePageNumberAndIndex(params.selectionEnd.pageNumber, params.selectionEnd.index),
+    ].join("");
+  }
+
+  // we never put search/section to the URL,
+  // yet we keep them in `locationParams`.
+  const finalParamsToSerialize: SearchParams = {
+    v: versionName,
+    rest: `${SEGMENT_SEPARATOR}${stringifiedParams.join(SEGMENT_SEPARATOR)}`,
+  };
+
+  return serializeSearchParams(finalParamsToSerialize);
+};
+
+function serializeSearchParams({ rest, ...searchParams }: SearchParams) {
+  const search = [];
+  for (const key of Object.keys(searchParams)) {
+    const val = searchParams[key as keyof typeof searchParams];
+    if (val) {
+      search.push(`${key}=${encodeURIComponent(val)}`);
+    }
+  }
+  return `${rest}${search.length > 0 ? `?${search.join("&")}` : ""}`;
+}

src/components/NoteManager/components/Note.tsx

@@ -55,7 +55,7 @@ export function Note({ note, active = false, onEditNote, onDeleteNote }: NotePro
   });
   const [noteContentError, setNoteContentError] = useState("");
 
-  const { setLocationParams } = useLocationContext();
+  const { setLocationParams, locationParams } = useLocationContext();
 
   const isEditable = note.source !== NoteSource.Remote;
 
@@ -115,7 +115,7 @@ export function Note({ note, active = false, onEditNote, onDeleteNote }: NotePro
     }
 
     setLocationParams({
-      version: note.original.version,
+      version: locationParams.version,
       selectionStart: note.original.selectionStart,
       selectionEnd: note.original.selectionEnd,
     });

src/components/NoteManager/components/NoteLink.tsx

@@ -1,5 +1,5 @@
 import { isSameBlock } from "@fluffylabs/links-metadata";
-import { type MouseEventHandler, useCallback, useContext, useEffect, useState } from "react";
+import { type MouseEventHandler, useCallback, useContext, useEffect, useMemo, useState } from "react";
 import { Tooltip } from "react-tooltip";
 import { CodeSyncContext, type ICodeSyncContext } from "../../CodeSyncProvider/CodeSyncProvider";
 import { type ILocationContext, LocationContext } from "../../LocationProvider/LocationProvider";
@@ -22,7 +22,7 @@ export function NoteLink({ note, onEditNote }: NoteLinkProps) {
   const { getSectionTitleAtSynctexBlock, getSubsectionTitleAtSynctexBlock } = useContext(
     CodeSyncContext,
   ) as ICodeSyncContext;
-  const { setLocationParams, locationParams } = useContext(LocationContext) as ILocationContext;
+  const { locationParams, getHashFromLocationParams } = useContext(LocationContext) as ILocationContext;
 
   const migrationFlag = !note.current.isUpToDate;
   const isEditable = note.source !== NoteSource.Remote;
@@ -42,29 +42,12 @@ export function NoteLink({ note, onEditNote }: NoteLinkProps) {
     })();
   }, [selectionStart, getSectionTitleAtSynctexBlock, getSubsectionTitleAtSynctexBlock]);
 
-  const handleNoteTitleClick = useCallback<MouseEventHandler>(
-    (e) => {
-      e.preventDefault();
-      setLocationParams({
-        ...locationParams,
-        selectionStart,
-        selectionEnd,
-      });
-    },
-    [selectionStart, selectionEnd, locationParams, setLocationParams],
-  );
-
-  const handleOriginalClick = useCallback<MouseEventHandler>(
-    (e) => {
-      e.preventDefault();
-      setLocationParams({
-        version: note.original.version,
-        selectionStart: note.original.selectionStart,
-        selectionEnd: note.original.selectionEnd,
-      });
-    },
-    [note, setLocationParams],
-  );
+  const handleNoteLinkClick = useCallback<MouseEventHandler>((e) => {
+    e.preventDefault();
+    const href = e.currentTarget.getAttribute("href");
+    if (!href) return;
+    window.location.hash = href;
+  }, []);
 
   const handleMigrateClick = useCallback<MouseEventHandler>(
     (e) => {
@@ -90,32 +73,53 @@ export function NoteLink({ note, onEditNote }: NoteLinkProps) {
     [locationParams, note, selectionStart, selectionEnd, onEditNote],
   );
 
+  const currentVersionLink = useMemo(
+    () =>
+      getHashFromLocationParams({
+        version: locationParams.version,
+        selectionStart: note.original.selectionStart,
+        selectionEnd: note.original.selectionEnd,
+      }),
+    [locationParams, note, getHashFromLocationParams],
+  );
+
+  const originalLink = useMemo(
+    () =>
+      getHashFromLocationParams({
+        version: note.original.version,
+        selectionStart: note.original.selectionStart,
+        selectionEnd: note.original.selectionEnd,
+      }),
+    [note, getHashFromLocationParams],
+  );
+
   const { section, subSection } = sectionTitle;
   return (
     <div className="note-link">
       {migrationFlag && (
         <a
-          href="#"
+          href={`#${originalLink}`}
           data-tooltip-id="note-link"
           data-tooltip-content="This note was created in a different version. Click here to see in original context."
           data-tooltip-place="top"
           className="icon default-link"
-          onClick={handleOriginalClick}
+          onClick={handleNoteLinkClick}
         >
           ⚠
         </a>
       )}
 
       <OutlineLink
+        href={`#${currentVersionLink}`}
         firstLevel
         title={subSection ? `${section} > ${subSection}` : section}
         number={`p. ${pageNumber} >`}
-        onClick={handleNoteTitleClick}
-        href="#"
+        onClick={handleNoteLinkClick}
       />
 
       {migrationFlag && isEditable && (
         <a
+          href="#"
           onClick={handleMigrateClick}
           data-tooltip-id="note-link"
           data-tooltip-content="Make sure the selection is accurate or adjust it in the current version and update the note."