SF-3629 Add support for links with titles (#3557)

Author: pmachapman

src/SIL.XForge.Scripture/ClientApp/src/_usx.scss

@@ -456,6 +456,10 @@ usx-note {
   }
 }
 
+usx-link {
+  text-decoration: underline dotted;
+}
+
 usx-ref {
   font-style: italic;
 }

src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-formats/quill-blot-value-types.ts

@@ -43,6 +43,11 @@ export interface Note extends UsxStyle {
   contents?: { ops: DeltaOperation[] };
 }
 
+export interface Link extends UsxStyle {
+  'link-href'?: string;
+  contents?: { ops: DeltaOperation[] };
+}
+
 export interface Figure extends UsxStyle {
   alt?: string;
   file: string;

src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-formats/quill-blots.spec.ts

@@ -7,6 +7,7 @@ import {
   CharInline,
   EmptyEmbed,
   FigureEmbed,
+  LinkEmbed,
   NoteEmbed,
   NoteThreadEmbed,
   NotNormalizedText,
@@ -248,6 +249,77 @@ describe('Quill blots', () => {
     });
   });
 
+  describe('LinkEmbed', () => {
+    it('should create link with title', () => {
+      const value = {
+        style: 'xt',
+        'link-href': 'GEN 1:1',
+        contents: {
+          ops: [{ insert: 'Genesis 1:1' }]
+        }
+      };
+      const node = LinkEmbed.create(value) as HTMLElement;
+
+      expect(node.innerText).toBe('Genesis 1:1');
+      expect(node.title).toBe('GEN 1:1');
+    });
+
+    it('should handle partial figure data', () => {
+      const value = {
+        contents: {
+          ops: [{ insert: 'Genesis 1:1' }]
+        }
+      };
+      const node = LinkEmbed.create(value as any) as HTMLElement;
+      expect(node.title).toBe('');
+    });
+
+    it('should handle contents with multiple ops', () => {
+      const value = {
+        'link-href': 'GEN 1:1',
+        contents: {
+          ops: [{ insert: 'Genesis' }, { insert: ' 1:1' }]
+        }
+      };
+      const node = LinkEmbed.create(value) as HTMLElement;
+
+      expect(node.innerText).toBe('Genesis 1:1');
+      expect(node.title).toBe('GEN 1:1');
+    });
+
+    it('should handle contents with unsupported ops', () => {
+      const value = {
+        'link-href': 'GEN 1:1',
+        contents: {
+          ops: [{ insert: 'Genesis 1:1' }, { insert: { ref: 'figure1' } }]
+        }
+      };
+      const node = LinkEmbed.create(value) as HTMLElement;
+
+      expect(node.innerText).toBe('Genesis 1:1');
+      expect(node.title).toBe('GEN 1:1');
+    });
+
+    it('should retrieve stored value', () => {
+      const value = {
+        style: 'xt',
+        'link-href': 'GEN 1:1',
+        contents: {
+          ops: [{ insert: 'Genesis 1:1' }]
+        }
+      };
+      const node = LinkEmbed.create(value as any) as HTMLElement;
+      expect(LinkEmbed.value(node)).toEqual(value as any);
+    });
+
+    it('should maintain DOM structure with empty fields', () => {
+      const value = {};
+      const node = LinkEmbed.create(value as any) as HTMLElement;
+      expect(node.innerText).toBe('');
+      expect(node.title).toBe('');
+    });
+  });
+
   describe('NoteEmbed', () => {
     it('should create note with caller and style', () => {
       const value = { caller: 'a', style: 'footnote' };
@@ -276,6 +348,66 @@ describe('Quill blots', () => {
       expect(node.title).toBe('note text');
     });
 
+    it('should set title from contents with a link', () => {
+      const value = {
+        caller: 'a',
+        contents: {
+          ops: [
+            { insert: 'note text ' },
+            { insert: { link: { style: 'xt', 'link-href': 'GEN: 1:1', contents: { ops: [{ insert: 'link text' }] } } } }
+          ]
+        }
+      };
+      const node = NoteEmbed.create(value) as HTMLElement;
+
+      expect(node.title).toBe('note text link text');
+    });
+
+    it('should set title from contents ignoring an link with no text', () => {
+      const value = {
+        caller: 'a',
+        contents: {
+          ops: [
+            { insert: 'note text' },
+            {
+              insert: {
+                link: {
+                  style: 'xt',
+                  'link-href': 'GEN: 1:1'
+                }
+              }
+            }
+          ]
+        }
+      };
+      const node = NoteEmbed.create(value) as HTMLElement;
+
+      expect(node.title).toBe('note text');
+    });
+
+    it('should set title from contents with a link containing an unsupported op', () => {
+      const value = {
+        caller: 'a',
+        contents: {
+          ops: [
+            { insert: 'note text ' },
+            {
+              insert: {
+                link: {
+                  style: 'xt',
+                  'link-href': 'GEN: 1:1',
+                  contents: { ops: [{ insert: 'link text' }, { insert: { ref: 'unsupported op' } }] }
+                }
+              }
+            }
+          ]
+        }
+      };
+      const node = NoteEmbed.create(value) as HTMLElement;
+
+      expect(node.title).toBe('note text link text');
+    });
+
     it('should handle null style', () => {
       const value = { caller: 'a' };
       const node = NoteEmbed.create(value) as HTMLElement;

src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-formats/quill-blots.ts

@@ -10,6 +10,7 @@ import {
   Book,
   Chapter,
   Figure,
+  Link,
   Note,
   NoteThread,
   Para,
@@ -272,10 +273,22 @@ export class NoteEmbed extends QuillEmbedBlot {
     }
     if (value.contents != null) {
       // ignore blank embeds (checked here as non-string insert)
-      node.title = value.contents.ops.reduce(
-        (text, op) => (typeof op.insert === 'string' ? text + op.insert : text),
-        ''
-      );
+      node.title = value.contents.ops.reduce((text, op) => {
+        if (typeof op.insert === 'string') {
+          return text + op.insert;
+        }
+
+        // Handle link inserts with nested contents
+        const link = op.insert?.link as Link | undefined;
+        if (link?.contents?.ops) {
+          const linkText = link.contents.ops
+            .map(innerOp => (typeof innerOp.insert === 'string' ? innerOp.insert : ''))
+            .join('');
+          return text + linkText;
+        }
+
+        return text;
+      }, '');
     }
     setUsxValue(node, value);
     return node;
@@ -388,6 +401,33 @@ export class FigureEmbed extends QuillEmbedBlot {
   }
 }
 
+export class LinkEmbed extends QuillEmbedBlot {
+  static blotName = 'link';
+  static tagName = 'usx-link';
+
+  static create(value: Link): Node {
+    const node = super.create(value) as HTMLElement;
+    if (value.style != null) {
+      node.setAttribute(customAttributeName('style'), value.style);
+    }
+    if (value['link-href'] != null) {
+      node.setAttribute(customAttributeName('link-href'), value['link-href']);
+      node.title = value['link-href'];
+    }
+    const contentsSpan = document.createElement('span');
+    contentsSpan.innerText =
+      value.contents?.ops.map(innerOp => (typeof innerOp.insert === 'string' ? innerOp.insert : '')).join('') ?? '';
+
+    node.appendChild(contentsSpan);
+    setUsxValue(node, value);
+    return node;
+  }
+
+  static value(node: HTMLElement): UsxStyle {
+    return getUsxValue(node);
+  }
+}
+
 export class UnmatchedEmbed extends QuillEmbedBlot {
   static blotName = 'unmatched';
   static tagName = 'usx-unmatched';

src/SIL.XForge.Scripture/ClientApp/src/app/shared/text/quill-editor-registration/quill-registrations.ts

@@ -28,6 +28,7 @@ import {
   CharInline,
   EmptyEmbed,
   FigureEmbed,
+  LinkEmbed,
   NoteEmbed,
   NoteThreadEmbed,
   NotNormalizedText,
@@ -57,6 +58,7 @@ export function registerScriptureFormats(formatRegistry: QuillFormatRegistryServ
     UnmatchedEmbed,
     ChapterEmbed,
     UnknownBlot,
+    LinkEmbed,
 
     // Inline Blots
     CharInline,

src/SIL.XForge.Scripture/usx-sf.rnc

@@ -134,7 +134,7 @@ Para =
             },
         attribute vid { xsd:string { pattern = "[A-Z1-4]{3} ?[a-z0-9\-,:]*" } }?,
         attribute status { text }?,
-        (Reference | Note | Char | Milestone | Figure | Verse | Break | Unmatched | text)+
+        (Reference | Note | Link | Char | Milestone | Figure | Verse | Break | Unmatched | text)+
     }
 Para.para.style.enum = (
       "restore" # Comment about when text was restored
@@ -430,7 +430,7 @@ ListChar.char.style.enum = (
 
 char.link =
     attribute link-href { xsd:string
-        { pattern = "(.*///?(.*/?)+)|((prj:[A-Za-z\-0-9]{3,8} )?[A-Z1-4]{3} \d+:\d+(\-\d+)?)|(#[^\s]+)" } }?, # The resource being linked to as a URI
+        { pattern = "(.*///?(.*/?)+)|((prj:[A-Za-z\-0-9]{3,8} )?[A-Z1-4]{3} \d+:\d+(\-\d+)?[ ]*)|(#[^\s]+)" } }?, # The resource being linked to as a URI
     attribute link-title { xsd:string }?, # Plain text describing the remote resource such as might be shown in a tooltip
     attribute link-id { xsd:string { pattern = "[\w_\-\.:]+" } }? # Unique identifier for this location in the text
 
@@ -483,15 +483,15 @@ Note =
         attribute style { "f" | "fe" | "ef" | "x" | "ex" },
         attribute caller { text },
         (attribute category { text })?,
-        (NoteChar | Unmatched | text )+
+        (NoteChar | Link | Unmatched | text )+
     }
 
 NoteChar =
     element char {
         attribute style { FootnoteChar.char.style.enum | CrossReferenceChar.char.style.enum },
         char.link?,
         char.closed?,
-        (Char | Reference | Unmatched | text)+
+        (Char | Link | Reference | Unmatched | text)+
     }
 FootnoteChar.char.style.enum = (
       "fr" # The origin reference for the footnote
@@ -544,6 +544,13 @@ Reference =
         text?
     }
 
+Link =
+    element link {
+        attribute style { CrossReferenceChar.char.style.enum },
+        char.link?,
+        text?
+    }
+
 Break =
     element optbreak { empty }
 

src/SIL.XForge.Scripture/usx-sf.xsd

@@ -143,6 +143,7 @@
       <xs:choice minOccurs="0" maxOccurs="unbounded">
         <xs:element ref="ref"/>
         <xs:element ref="note"/>
+        <xs:element ref="link"/>
         <xs:group ref="Char"/>
         <xs:element ref="ms"/>
         <xs:element ref="figure"/>
@@ -563,7 +564,7 @@
     <xs:attribute name="link-href">
       <xs:simpleType>
         <xs:restriction base="xs:string">
-          <xs:pattern value="(.*///?(.*/?)+)|((prj:[A-Za-z\-0-9]{3,8} )?[A-Z1-4]{3} \d+:\d+(\-\d+)?)|(#[^\s]+)"/>
+          <xs:pattern value="(.*///?(.*/?)+)|((prj:[A-Za-z\-0-9]{3,8} )?[A-Z1-4]{3} \d+:\d+(\-\d+)?[ ]*)|(#[^\s]+)"/>
         </xs:restriction>
       </xs:simpleType>
     </xs:attribute>
@@ -726,6 +727,7 @@
     <xs:complexType mixed="true">
       <xs:choice minOccurs="0" maxOccurs="unbounded">
         <xs:group ref="NoteChar"/>
+        <xs:element ref="link"/>
         <xs:element ref="unmatched"/>
       </xs:choice>
       <xs:attribute name="style" use="required">
@@ -749,6 +751,7 @@
         <xs:complexType mixed="true">
           <xs:choice minOccurs="0" maxOccurs="unbounded">
             <xs:group ref="Char"/>
+            <xs:element ref="link"/>
             <xs:element ref="ref"/>
             <xs:element ref="unmatched"/>
           </xs:choice>
@@ -820,6 +823,12 @@
       </xs:attribute>
     </xs:complexType>
   </xs:element>
+  <xs:element name="link">
+    <xs:complexType mixed="true">
+      <xs:attribute name="style" use="required" type="CrossReferenceChar.char.style.enum"/>
+      <xs:attributeGroup ref="char.link"/>
+    </xs:complexType>
+  </xs:element>
   <xs:element name="optbreak">
     <xs:complexType/>
   </xs:element>

tools/Roundtrip/Program.cs

@@ -258,6 +258,14 @@ void Roundtrip(string usfm, string fileName, string path, RoundtripMethod roundt
                 true
             );
         }
+
+        // Output the USX if requested
+        if (outputAllFiles)
+        {
+            actualUsx.Save(
+                Path.Join("output", $"{Path.GetFileName(path)}-{Path.GetFileNameWithoutExtension(fileName)}-usx.xml")
+            );
+        }
     }
     else
     {