From 010ca30999a2ebd88d199281ca9ddbbea9ebfe48 Mon Sep 17 00:00:00 2001
From: SilentSpike <SilentSpike100+Github@gmail.com>
Date: Mon, 3 Feb 2020 11:39:40 +0000
Subject: [PATCH] Show more Osmose strings in the UI

Also only add sections where appropriate
---
 data/core.yaml               |  2 ++
 dist/locales/en.json         |  2 ++
 modules/services/osmose.js   |  2 ++
 modules/ui/osmose_details.js | 62 ++++++++++++++++++++++++++----------
 4 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/data/core.yaml b/data/core.yaml
index 98d36b3cc..92e5d3c73 100644
--- a/data/core.yaml
+++ b/data/core.yaml
@@ -830,6 +830,8 @@ en:
     osmose:
       title: Osmose Issue
       elems_title: Issue Elements
+      fix_title: Fix Guidelines
+      trap_title: Common Pitfalls
       details:
         values: Issue Values
         tags: Issue Tags
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 819618a99..2e2d4e314 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -1034,6 +1034,8 @@
             "osmose": {
                 "title": "Osmose Issue",
                 "elems_title": "Issue Elements",
+                "fix_title": "Fix Guidelines",
+                "trap_title": "Common Pitfalls",
                 "details": {
                     "values": "Issue Values",
                     "tags": "Issue Tags",
diff --git a/modules/services/osmose.js b/modules/services/osmose.js
index 58a777526..6abd37fea 100644
--- a/modules/services/osmose.js
+++ b/modules/services/osmose.js
@@ -258,6 +258,8 @@ export default {
                 // If string exists, value is an object with key 'auto' for string
                 let { title, detail, trap, fix, example } = item.class[k];
                 if (title) issueStrings.title = title.auto;
+                // TODO: Replace \[(.+?)\]\((.+?)\) pattern with <a href="\2">\1</a>
+                // TODO: Replace `(.+?)` with some sort of code styling
                 if (detail) issueStrings.detail = detail.auto;
                 if (trap) issueStrings.trap = trap.auto;
                 if (fix) issueStrings.fix = fix.auto;
diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js
index 788dd4d3e..1aad58a7e 100644
--- a/modules/ui/osmose_details.js
+++ b/modules/ui/osmose_details.js
@@ -10,17 +10,16 @@ import { utilDisplayName, utilEntityOrMemberSelector } from '../util';
 
 
 export function uiOsmoseDetails(context) {
-    var _error;
+    let _error;
+    const unknown = t('inspector.unknown');
 
 
-    function issueDetail(d) {
-        var unknown = t('inspector.unknown');
-
+    function issueString(d, type) {
         if (!d) return unknown;
 
         // Issue description supplied by Osmose
         var s = services.osmose.getStrings(d.error_type);
-        return ('detail' in s) ? s.detail : unknown;
+        return (type in s) ? s[type] : unknown;
     }
 
 
@@ -51,22 +50,50 @@ export function uiOsmoseDetails(context) {
         descriptionEnter
             .append('div')
             .attr('class', 'error-details-description-text')
-            .html(issueDetail);
+            .html(d => issueString(d, 'detail'));
 
         // Elements (populated later as data is requested)
-        descriptionEnter
-            .append('h4')
-            .attr('class', 'error-details-subtitle')
-            .text(() => t('QA.osmose.elems_title'));
+        const detailsDiv = descriptionEnter.append('div');
 
-        var elementList = descriptionEnter
-            .append('ul')
-            .attr('class', 'error-details-elements');
+        // Suggested Fix (musn't exist for every issue type)
+        if (issueString(_error, 'fix') !== unknown) {
+            descriptionEnter
+                .append('h4')
+                .attr('class', 'error-details-subtitle')
+                .text(() => t('QA.osmose.fix_title'));
+
+            descriptionEnter
+                .append('div')
+                .attr('class', 'error-details-description-text')
+                .html(d => issueString(d, 'fix'));
+        }
+
+        // Common Pitfalls (musn't exist for every issue type)
+        if (issueString(_error, 'trap') !== unknown) {
+            descriptionEnter
+                .append('h4')
+                .attr('class', 'error-details-subtitle')
+                .text(() => t('QA.osmose.trap_title'));
+
+            descriptionEnter
+                .append('div')
+                .attr('class', 'error-details-description-text')
+                .html(d => issueString(d, 'trap'));
+        }
 
         services.osmose.loadErrorDetail(_error, (err, d) => {
-            if (d.elems === undefined) return;
+            // No details to add if there are no associated issue elements
+            if (!d.elems || d.elems.length === 0) return;
 
-            elementList.selectAll('.error_entity_link')
+            detailsDiv
+                .append('h4')
+                .attr('class', 'error-details-subtitle')
+                .text(() => t('QA.osmose.elems_title'));
+
+            detailsDiv
+                .append('ul')
+                .attr('class', 'error-details-elements')
+                .selectAll('.error_entity_link')
                 .data(d.elems)
                 .enter()
                 .append('li')
@@ -122,16 +149,17 @@ export function uiOsmoseDetails(context) {
                     }
                 });
 
+            // TODO: Show the dynamic subtitle string directly once langs parameter is added
             // Things like keys and values are dynamic details
             const special = { tags: true, values: true, chars: true, sug_tags: true };
             for (const type in special) {
                 if (type in d) {
-                    descriptionEnter
+                    detailsDiv
                         .append('h4')
                         .attr('class', 'error-details-subtitle')
                         .text(() => t(`QA.osmose.details.${type}`));
 
-                    descriptionEnter
+                    detailsDiv
                         .append('ul')
                         .attr('class', 'error-details-list')
                         .selectAll('li')