[settings-view] Fix rendering of setting descriptions with line breaks… (#1620)

…so that each section is wrapped in a P tag when appropriate.

Author: savetheclocktower

packages/settings-view/lib/rich-description.js

@@ -1,6 +1,6 @@
 
 module.exports = {
-  getSettingDescription (keyPath) {
+  getSettingDescription(keyPath) {
     const schema = atom.config.getSchema(keyPath)
     let description = ''
     if (schema && schema.description) {
@@ -11,13 +11,20 @@ module.exports = {
     if (atom.i18n.isAutoTranslateLabel(description)) {
       description = atom.i18n.translateLabel(description);
     }
-    
-    return atom.ui.markdown.render(
+
+    let contents = atom.ui.markdown.render(
       description,
       {
         useTaskCheckbox: false,
         disableMode: "strict",
       }
-    ).replace(/<p>(.*)<\/p>/, "$1").trim();
+    )
+
+    // If the setting has no internal paragraph breaks, strip it of its
+    // surrounding `p` tag. Otherwise keep the `p`.
+    if ((contents.match(/<p>/g)?.length ?? 0) <= 1) {
+      contents = contents.replace(/<p>(.*)<\/p>/, "$1").trim()
+    }
+    return contents
   }
 }

packages/settings-view/lib/settings-panel.js

@@ -490,6 +490,7 @@ function elementForOptions (namespace, name, value, {radio = false}) {
 
   const label = document.createElement('label')
   label.classList.add('control-label')
+  label.dataset.settingKey = keyPath
 
   const titleDiv = document.createElement('div')
   titleDiv.classList.add('setting-title')
@@ -515,6 +516,7 @@ function elementForCheckbox (namespace, name, value) {
 
   const label = document.createElement('label')
   label.for = keyPath
+  label.dataset.settingKey = keyPath
 
   const input = document.createElement('input')
   input.id = keyPath
@@ -544,6 +546,7 @@ function elementForColor (namespace, name, value) {
 
   const label = document.createElement('label')
   label.for = keyPath
+  label.dataset.settingKey = keyPath
 
   const input = document.createElement('input')
   input.id = keyPath
@@ -572,6 +575,7 @@ function elementForEditor (namespace, name, value) {
 
   const label = document.createElement('label')
   label.classList.add('control-label')
+  label.dataset.settingKey = keyPath
 
   const titleDiv = document.createElement('div')
   titleDiv.classList.add('setting-title')
@@ -607,6 +611,7 @@ function elementForArray (namespace, name, value) {
 
   const label = document.createElement('label')
   label.classList.add('control-label')
+  label.dataset.settingKey = keyPath
 
   const titleDiv = document.createElement('div')
   titleDiv.classList.add('setting-title')

packages/settings-view/spec/.eslintrc.js

@@ -5,7 +5,8 @@ module.exports = {
     node: true
   },
   globals: {
-    waitsForPromise: true
+    waitsForPromise: true,
+    advanceClock: true
   },
   rules: {
     "node/no-missing-require": "off",

packages/settings-view/spec/settings-panel-spec.js

@@ -465,4 +465,57 @@ describe("SettingsPanel", () => {
       });
     });
   });
+
+  describe('settings rendering', () => {
+    beforeEach(() => {
+      const config = {
+        type: 'object',
+        properties: {
+          troz: {
+            title: 'troz',
+            // Testing paragraph breaks inside a description.
+            description: 'The troz setting.\n\nIgnored unless `zort` is also `true`.',
+            type: 'string',
+            default: 'troz',
+          },
+          minMax: {
+            name: 'minMax',
+            title: 'Min max',
+            description: 'The minMax setting',
+            type: 'integer',
+            default: 10,
+            minimum: 1,
+            maximum: 100
+          },
+          commaValueArray: {
+            name: 'commaValueArray',
+            title: 'Comma value in array',
+            description: 'An array with a comma value',
+            type: 'array',
+            default: []
+          }
+        }
+      };
+
+      atom.config.setSchema('foo', config);
+      settingsPanel = new SettingsPanel({namespace: 'foo', includeTitle: false});
+    });
+
+    it('renders a setting description as Markdown', () => {
+      // A setting with internal paragraph breaks should end up with two P
+      // elements…
+      const trozDescription = settingsPanel.element.querySelector(
+        `label[data-setting-key="foo.troz"] > .setting-description`
+      );
+      expect(trozDescription.querySelectorAll('p').length).toBe(2);
+
+      // …but a setting without any paragraph breaks should have its outer P
+      // element stripped.
+      const minMaxDescription = settingsPanel.element.querySelector(
+        `label[data-setting-key="foo.minMax"] > .setting-description`
+      );
+      expect(minMaxDescription.querySelectorAll('p').length).toBe(0);
+    });
+  });
+
 });