DEV: Migrate icons and links settings to new objects setting type (#36)

Since discourse/discourse@a440e15, we have started to support objects typed theme setting so we are switching this theme component to use it instead as it provides a much better UX for configuring the settings required for the theme component.

.discourse-compatibility

@@ -1,3 +1,4 @@
+< 3.3.0.beta2-dev: e64cfa531121f2cf152901f5b2a20512a5ee2c63
 < 3.3.0.beta1-dev: 35b0c2da37318f502bb7069ee8e3565eb3ba5dbf
 3.1.999: e2cc21b33261ffeb32030e8ab5ea92775c911fbc
 < 3.2.0.beta1-dev: b36e2a5199db23e5f05374640b9690573a08b352

javascripts/discourse/components/brand-header-contents.gjs

@@ -0,0 +1,79 @@
+import Component from "@glimmer/component";
+import { inject as service } from "@ember/service";
+import dIcon from "discourse-common/helpers/d-icon";
+
+export default class BrandHeaderContents extends Component {
+  @service site;
+
+  get shouldShow() {
+    return !this.site.mobileView || settings.show_bar_on_mobile;
+  }
+
+  get brandLogo() {
+    const mobileView = this.site.mobileView;
+    const mobileLogoUrl = settings.mobile_logo_url || "";
+    const showMobileLogo = mobileView && mobileLogoUrl.length > 0;
+    const logoUrl = settings.logo_url || "";
+    const title = settings.brand_name;
+
+    return {
+      url: showMobileLogo ? mobileLogoUrl : logoUrl,
+      title,
+    };
+  }
+
+  get hasIcons() {
+    return settings.icons.length > 0;
+  }
+
+  get hasLinks() {
+    return settings.links.length > 0;
+  }
+
+  <template>
+    <div class="title">
+      <a href={{settings.website_url}}>
+        {{#if this.brandLogo.url}}
+          <img
+            id="brand-logo"
+            class="logo-big"
+            src={{this.brandLogo.url}}
+            title={{this.brandLogo.title}}
+          />
+        {{else}}
+          <h2 id="#brand-text-logo" class="text-logo">
+            {{settings.brand_name}}
+          </h2>
+        {{/if}}
+      </a>
+    </div>
+
+    {{#if this.hasLinks}}
+      <nav class="links">
+        <ul class="nav {{if this.shouldShow 'nav-pills'}}">
+          {{#each settings.links as |tl|}}
+            <li>
+              <a href={{tl.url}} target={{tl.target}}>
+                {{tl.text}}
+              </a>
+            </li>
+          {{/each}}
+        </ul>
+      </nav>
+    {{/if}}
+
+    {{#if this.hasIcons}}
+      <div class="panel">
+        <ul class="icons">
+          {{#each settings.icons as |il|}}
+            <li>
+              <a href={{il.url}} target={{il.target}}>
+                {{dIcon il.icon_name}}
+              </a>
+            </li>
+          {{/each}}
+        </ul>
+      </div>
+    {{/if}}
+  </template>
+}

locales/en.yml

@@ -0,0 +1,29 @@
+en:
+  theme_metadata:
+    settings:
+      links:
+        description: Text links to be displayed in the header
+        schema:
+          properties:
+            text:
+              label: Text
+              description: The text to display for the link
+            url:
+              label: URL
+              description: The URL to link to
+            target:
+              label: Target
+              description: The target attribute for the link
+      icons:
+        description: Icon links to be displayed in the header
+        schema:
+          properties:
+            icon_name:
+              label: Icon
+              description: The name of the FontAwesome 5 icon to display for the link
+            url:
+              label: URL
+              description: The URL to link to
+            target:
+              label: Target
+              description: The target attribute for the link

migrations/settings/0001-migrate-links-setting.js

@@ -0,0 +1,31 @@
+export default function migrate(settings) {
+  const oldSetting = settings.get("links");
+
+  if (oldSetting) {
+    const newSetting = oldSetting.split("|").map((link) => {
+      let [name, url, target] = link.split(",").map((s) => s.trim());
+
+      if (["_blank", "_self", "_parent", "_top"].indexOf(target) === -1) {
+        target = "_blank";
+      }
+
+      const newLink = {
+        name,
+        url,
+        target
+      }
+
+      Object.keys(newLink).forEach((key) => {
+        if (newLink[key] === undefined) {
+          delete newLink[key];
+        }
+      });
+
+      return newLink;
+    })
+
+    settings.set("links", newSetting);
+  }
+
+  return settings;
+}

migrations/settings/0002-migrate-icons-setting.js

@@ -0,0 +1,31 @@
+export default function migrate(settings) {
+  const oldSetting = settings.get("icons");
+
+  if (oldSetting) {
+    const newSetting = oldSetting.split("|").map((link) => {
+      let [iconName, url, target] = link.split(",").map((s) => s.trim());
+
+      if (["_blank", "_self", "_parent", "_top"].indexOf(target) === -1) {
+        target = "_blank";
+      }
+
+      const newLink = {
+        icon_name: iconName,
+        url,
+        target
+      }
+
+      Object.keys(newLink).forEach((key) => {
+        if (newLink[key] === undefined) {
+          delete newLink[key];
+        }
+      });
+
+      return newLink;
+    })
+
+    settings.set("icons", newSetting);
+  }
+
+  return settings;
+}

settings.yaml

@@ -6,16 +6,63 @@ logo_url:
   default: ""
 mobile_logo_url:
   default: ""
+
 links:
-  type: list
-  list_type: simple
-  default: ""
-  description: "Format: name,url,target (optional)"
+  type: objects
+  default: []
+  schema:
+    name: link
+    properties:
+      text:
+        type: string
+        required: true
+        validations:
+          min: 1
+          max: 100
+      url:
+        type: string
+        required: true
+        validations:
+          min: 1
+          max: 2048
+          url: true
+      target:
+        type: enum
+        default: _blank
+        choices:
+          - _blank
+          - _self
+          - _parent
+          - _top
+
 icons:
-  type: list
-  list_type: simple
-  default: ""
-  description: "Format: icon-name,url,target (optional)"
+  type: objects
+  default: []
+  schema:
+    name: icon
+    properties:
+      icon_name:
+        type: string
+        required: true
+        validations:
+          min: 1
+          max: 100
+      url:
+        type: string
+        required: true
+        validations:
+          min: 1
+          max: 2048
+          url: true
+      target:
+        type: enum
+        default: _blank
+        choices:
+          - _blank
+          - _self
+          - _parent
+          - _top
+
 custom_font_awesome_icons:
   type: list
   list_type: simple

spec/system/viewing_brand_header_spec.rb

@@ -27,13 +27,20 @@
 
     theme.update_setting(
       :links,
-      "First Link,http://some.url.com/first|Second Link,http://some.url.com/second,_blank",
+      [
+        { text: "First Link", url: "http://some.url.com/first", target: "_blank" },
+        { text: "Second Link", url: "http://some.url.com/second", target: "_self" },
+      ],
     )
 
     theme.update_setting(
       :icons,
-      "wrench,http://some.url.com/some-wrench-link|pencil,http://some.url.com/some-pencil-link,_blank",
+      [
+        { icon_name: "wrench", url: "http://some.url.com/some-wrench-link", target: "_self" },
+        { icon_name: "pencil", url: "http://some.url.com/some-pencil-link", target: "_blank" },
+      ],
     )
+
     theme.save!
 
     visit("/")
@@ -44,10 +51,12 @@
       'img#brand-logo[title="some name"][src="http://some.url.com/logo.png"]',
     )
 
-    expect(page).to have_link("First Link", href: "http://some.url.com/first")
-    expect(page).to have_link("Second Link", href: "http://some.url.com/second", target: "_blank")
+    expect(page).to have_link("First Link", href: "http://some.url.com/first", target: "_blank")
+    expect(page).to have_link("Second Link", href: "http://some.url.com/second", target: "_self")
 
-    expect(page).to have_selector('a[href="http://some.url.com/some-wrench-link"] .d-icon-wrench')
+    expect(page).to have_selector(
+      'a[href="http://some.url.com/some-wrench-link"][target="_self"] .d-icon-wrench',
+    )
 
     expect(page).to have_selector(
       'a[href="http://some.url.com/some-pencil-link"][target="_blank"] .d-icon-pencil',