Skip to content

Commit ff3c261

Browse files
jjaffeuxjjaffeux
authored
DEV: migrates Header_links setting to an object (#26)
Prior to this commit the setting was only a string split at runtime on a | separator. This is now using our object settings and can be modifier more easily by ours users using the object editor. This commit also - renames the setting from Header_links to header_links - fixes a bug where we were rendering, and only hiding mobile icons on desktop when using (vmo) which was causing the "last-custom-icon" class to be incorrectly set on an hidden icon - adds a system spec
1 parent 952811b commit ff3c261

File tree

6 files changed

+228
-36
lines changed

6 files changed

+228
-36
lines changed

‎javascripts/discourse/initializers/initialize-for-header-icon-links.gjs

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { dasherize } from "@ember/string";
2+
import concatClass from "discourse/helpers/concat-class";
23
import { withPluginApi } from "discourse/lib/plugin-api";
34
import icon from "discourse-common/helpers/d-icon";
45
import isValidUrl from "../lib/isValidUrl";
@@ -19,31 +20,39 @@ export default {
1920
initialize() {
2021
withPluginApi("0.8.41", (api) => {
2122
try {
22-
const splitLinks = settings.Header_links.split("|").filter(Boolean);
23+
const site = api.container.lookup("service:site");
24+
let links = settings.header_links;
25+
if (site.mobileView) {
26+
links = links.filter(
27+
(link) => link.view === "vmo" || link.view === "vdm"
28+
);
29+
} else {
30+
links = links.filter(
31+
(link) => link.view === "vdo" || link.view === "vdm"
32+
);
33+
}
2334

24-
splitLinks.forEach((link, index, links) => {
25-
const fragments = link.split(",").map((fragment) => fragment.trim());
26-
const title = fragments[0];
27-
const iconTemplate = buildIcon(fragments[1], title);
28-
const href = fragments[2];
29-
const className = `header-icon-${dasherize(fragments[0])}`;
30-
const viewClass = fragments[3].toLowerCase();
31-
const target = fragments[4].toLowerCase() === "blank" ? "_blank" : "";
32-
const rel = target ? "noopener" : "";
35+
links.forEach((link, index) => {
36+
const iconTemplate = buildIcon(link.icon, link.title);
37+
const className = `header-icon-${dasherize(link.title)}`;
38+
const target = link.target === "blank" ? "_blank" : "";
39+
const rel = link.target ? "noopener" : "";
3340
const isLastLink =
34-
link === links[links.length - 1] ? "last-custom-icon" : "";
41+
index === links.length - 1 ? "last-custom-icon" : "";
3542

3643
const iconComponent = <template>
3744
<li
38-
class="custom-header-icon-link
39-
{{className}}
40-
{{viewClass}}
41-
{{isLastLink}}"
45+
class={{concatClass
46+
"custom-header-icon-link"
47+
className
48+
link.view
49+
isLastLink
50+
}}
4251
>
4352
<a
4453
class="btn no-text icon btn-flat"
45-
href={{href}}
46-
title={{title}}
54+
href={{link.url}}
55+
title={{link.title}}
4756
target={{target}}
4857
rel={{rel}}
4958
>
@@ -54,7 +63,9 @@ export default {
5463

5564
const beforeIcon = ["chat", "search", "hamburger", "user-menu"];
5665

57-
api.headerIcons.add(title, iconComponent, { before: beforeIcon });
66+
api.headerIcons.add(link.title, iconComponent, {
67+
before: beforeIcon,
68+
});
5869
});
5970
} catch (error) {
6071
// eslint-disable-next-line no-console

‎locales/en.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
en:
2+
theme_metadata:
3+
settings:
4+
header_links:
5+
description: Custom links to be displayed in the header
6+
schema:
7+
properties:
8+
title:
9+
label: Title
10+
description: The title attribute for the link
11+
icon:
12+
label: Icon
13+
description: The icon used for the link
14+
url:
15+
label: URL
16+
description: The URL for the link
17+
view:
18+
label: View
19+
description: |
20+
vdm = desktop and mobile
21+
vdo = desktop only
22+
vmo = mobile only
23+
target:
24+
label: Target
25+
description: |
26+
blank = opens in a new tab
27+
self = opens in the same tab

‎migrations/settings/0001-migrate-to-object-settings.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
export default function migrate(settings) {
2+
const oldSetting = settings.get("Header_links");
3+
4+
if (!oldSetting) {
5+
return settings;
6+
}
7+
8+
const newSetting = oldSetting.split("|").map((link) => {
9+
const [title, icon, url, view, target] = link
10+
.split(",")
11+
.map((s) => s.trim());
12+
13+
const newLink = {
14+
title,
15+
icon,
16+
url,
17+
view,
18+
target,
19+
};
20+
21+
Object.keys(newLink).forEach((key) => {
22+
if (newLink[key] === undefined) {
23+
delete newLink[key];
24+
}
25+
});
26+
27+
return newLink;
28+
});
29+
30+
settings.delete("Header_links");
31+
32+
settings.set("header_links", newSetting);
33+
return settings;
34+
}

‎settings.yml

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,3 @@
1-
Header_links:
2-
type: list
3-
default: "Desktop mobile link, fab-facebook, https://facebook.com, vdm, blank|Mobile-only link, fab-twitter, https://twitter.com, vmo, blank"
4-
description:
5-
en: "Comma delimited in this order: <br>
6-
<b>title, icon, URL, view, target</b>
7-
<br>
8-
<br>
9-
<b>title</b>: the desired title you want the link to have (when hovered). <br>
10-
<b>icon</b>: the icon you want the link to have. <br>
11-
<b>URL</b>: the desired URL you want the link to point to. <br>
12-
<b>View</b>:
13-
<ul>
14-
<li>visible on both desktop and mobile devices (vdm)</li>
15-
<li>Desktop only (vdo).</li>
16-
<li>mobile only (vmo).</li>
17-
</ul>
18-
<b>Target</b>: same tab (self) or new tab (blank)."
191
add_whitespace:
202
type: bool
213
default: false
@@ -27,3 +9,49 @@ Svg_icons:
279
default: "fab-facebook|fab-twitter"
2810
description:
2911
en: "Include FontAwesome 5 icon classes for each icon used in the list."
12+
header_links:
13+
type: objects
14+
default:
15+
- title: "Desktop and mobile link"
16+
icon: "fab-facebook"
17+
url: "https://facebook.com"
18+
view: "vdm"
19+
target: "blank"
20+
- title: "Mobile-only link"
21+
icon: "fab-twitter"
22+
url: "https://twitter.com"
23+
view: "vmo"
24+
target: "blank"
25+
schema:
26+
name: "link"
27+
properties:
28+
title:
29+
type: string
30+
required: true
31+
validations:
32+
min_length: 1
33+
max_length: 1000
34+
icon:
35+
type: string
36+
required: true
37+
validations:
38+
min_length: 1
39+
max_length: 100
40+
url:
41+
type: string
42+
required: true
43+
validations:
44+
min_length: 1
45+
max_length: 2048
46+
url: true
47+
view:
48+
type: enum
49+
choices:
50+
- vdm
51+
- vdo
52+
- vmo
53+
target:
54+
type: enum
55+
choices:
56+
- blank
57+
- self

‎spec/system/discourse_icon_header_links_spec.rb

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
RSpec.describe "Discourse icon header links", system: true do
2+
let(:theme) { Fabricate(:theme) }
3+
let!(:component) { upload_theme_component(parent_theme_id: theme.id) }
4+
5+
before { theme.set_default! }
6+
7+
context "when in desktop" do
8+
it "renders the correct icon" do
9+
visit("/")
10+
11+
expect(
12+
page.find(
13+
".custom-header-icon-link.header-icon-desktop-and-mobile-link.vdm.last-custom-icon",
14+
),
15+
).to have_link(
16+
title: "Desktop and mobile link",
17+
href: "https://facebook.com",
18+
target: "_blank",
19+
)
20+
expect(
21+
page.find(
22+
".custom-header-icon-link.header-icon-desktop-and-mobile-link.vdm.last-custom-icon",
23+
),
24+
).to have_selector(".d-icon-fab-facebook")
25+
end
26+
end
27+
28+
context "when in mobile", mobile: true do
29+
it "renders the correct icon" do
30+
visit("/")
31+
32+
expect(
33+
page.find(".custom-header-icon-link.header-icon-desktop-and-mobile-link.vdm"),
34+
).to have_link(
35+
title: "Desktop and mobile link",
36+
href: "https://facebook.com",
37+
target: "_blank",
38+
)
39+
expect(
40+
page.find(".custom-header-icon-link.header-icon-desktop-and-mobile-link.vdm"),
41+
).to have_selector(".d-icon-fab-facebook")
42+
43+
expect(
44+
page.find(".custom-header-icon-link.header-icon-mobile-only-link.vmo.last-custom-icon"),
45+
).to have_link(title: "Mobile-only link", href: "https://twitter.com", target: "_blank")
46+
expect(
47+
page.find(".custom-header-icon-link.header-icon-mobile-only-link.vmo.last-custom-icon"),
48+
).to have_selector(".d-icon-fab-twitter")
49+
end
50+
end
51+
end

‎test/unit/migrations/settings/0001-migrate-to-object-settings-test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { module, test } from "qunit";
2+
import migrate from "../../../../migrations/settings/0001-migrate-to-object-settings";
3+
4+
module(
5+
"Unit | Migrations | Settings | 0001-migrate-to-object-settings",
6+
function () {
7+
test("migrate", function (assert) {
8+
const settings = new Map(
9+
Object.entries({
10+
Header_links:
11+
"Desktop and mobile link, fab-facebook, https://facebook.com, vdm, blank|Mobile-only link, fab-twitter, https://twitter.com, vmo, blank",
12+
})
13+
);
14+
15+
const result = migrate(settings);
16+
17+
const expectedResult = new Map(
18+
Object.entries({
19+
header_links: [
20+
{
21+
icon: "fab-facebook",
22+
title: "Desktop and mobile link",
23+
url: "https://facebook.com",
24+
view: "vdm",
25+
target: "blank",
26+
},
27+
{
28+
icon: "fab-twitter",
29+
title: "Mobile-only link",
30+
url: "https://twitter.com",
31+
view: "vmo",
32+
target: "blank",
33+
},
34+
],
35+
})
36+
);
37+
38+
assert.deepEqual(Array.from(result), Array.from(expectedResult));
39+
});
40+
}
41+
);

0 commit comments

Comments
 (0)