Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow custom slugs #4918

Merged
merged 13 commits into from
Feb 19, 2021
Merged

Allow custom slugs #4918

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8e0bc8a
Update JSON scheme, build & test scripts for custom slugs
ericcornelissen Feb 3, 2021
6791e47
Example: invalid custom slug
ericcornelissen Feb 3, 2021
63dd392
Example: valid slug, wrong filename
ericcornelissen Feb 3, 2021
a37f73c
Example: custom slug, correct filename
ericcornelissen Feb 3, 2021
3aa70d9
Revert to 8e0bc8a85caa86e3732fa4a56909e6b02785fa01
ericcornelissen Feb 3, 2021
b2a2d22
Update index.html for custom slugs
ericcornelissen Feb 3, 2021
dd629af
Example: custom slug for 1Password
ericcornelissen Feb 3, 2021
664717f
Revert to b2a2d22393d4017cf93d3d76e01ea6f3c8d4acd0
ericcornelissen Feb 3, 2021
d99028f
Make NPM package API backwards compatible with custom slugs
ericcornelissen Feb 3, 2021
14a1aa9
Example: two icons with same tile
ericcornelissen Feb 3, 2021
e16681b
Revert to d99028f7a97afe5c3fbc59603a3c4674d163e992
ericcornelissen Feb 3, 2021
0550047
Update NodeJS usage documentation
ericcornelissen Feb 3, 2021
c77d5f3
Merge branch 'develop' into custom-slugs
ericcornelissen Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .jsonlintschema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
"type": "string",
"required": true
},
"slug": {
"description": "The brand name slug (used as filename in icons/)",
"type": "string",
"pattern": "^[a-z0-9\\-]+_[a-z0-9\\-]+$",
"required": false
},
"hex": {
"description": "The icons color, as HEX (without #)",
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ The API can then be used as follows:
```javascript
const simpleIcons = require('simple-icons');

console.log(simpleIcons.get('Simple Icons'));
console.log(simpleIcons.get('simpleicons')); // Use the filename of the SVG

/*
{
Expand Down
3 changes: 3 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@
{% assign filename = filename | replace: ".", "-dot-" %}
{% assign filename = filename | replace: "&", "-and-" %}
{% assign filename = filename | replace: " ", "" | replace: "!", "" | replace: ":", "" | replace: "’", "" | replace: "'", "" | replace: "°", "" %}
{% if icon.slug %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be an if/else statement (assuming Liquid supports them), to avoid all those replacements if slug is present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Liquid does support if/else statements and we can write it using one, this is just the lazy version (from the programmer's perspective): Liquid will first do all the replacements on the filename - for all icons - and if the icon has a slug specified then the filename is replaced by the slug. It's just wasting a bunch of CPU cycles for icons that have a custom slug.

I'd be happy to change it if you prefer 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, given that we'll have switched over to the new site before we have enough slug entries for this to be a major issue, I don't think it's such a big deal if we don't update it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my reasoning for this going with this easier but more obscure solution as well 😄

{% assign filename = icon.slug %}
{% endif %}

{% assign hex = icon.hex %}
{% assign hexCharacter1 = hex | slice: 0, 1 %}
Expand Down
10 changes: 7 additions & 3 deletions scripts/build-package.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ function escape(value) {
return value.replace(/(?<!\\)'/g, "\\'");
}
function iconToKeyValue(icon) {
const iconTitle = escape(icon.title);
return `'${iconTitle}':${iconToObject(icon)}`;
let iconName = escape(icon.title);
if (icon.slug !== titleToFilename(icon.title)) {
iconName = icon.slug;
}

return `'${iconName}':${iconToObject(icon)}`;
}
function iconToObject(icon) {
return util.format(iconObjectTemplate,
Expand All @@ -47,7 +51,7 @@ function iconToObject(icon) {
// 'main'
const icons = [];
data.icons.forEach(icon => {
const filename = titleToFilename(icon.title);
const filename = icon.slug || titleToFilename(icon.title);
icon.svg = fs.readFileSync(`${iconsDir}/${filename}.svg`, UTF8).replace(/\r?\n/, '');
icon.slug = filename;
icons.push(icon);
Expand Down
8 changes: 7 additions & 1 deletion scripts/templates/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ Object.defineProperty(icons, "get", {
var normalizedName = targetName.toLowerCase();
for (var iconName in icons) {
var icon = icons[iconName];
if (icon.title.toLowerCase() === normalizedName || icon.slug === normalizedName) {
if (icon.slug === normalizedName) {
return icon;
}
}
for (var iconName in icons) {
var icon = icons[iconName];
if (icon.title.toLowerCase() === normalizedName) {
return icon;
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/icons.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { icons } = require('../_data/simple-icons.json');
const { titleToFilename } = require('../scripts/utils.js');

icons.forEach(icon => {
const filename = titleToFilename(icon.title);
const filename = icon.slug || titleToFilename(icon.title);
const subject = require(`../icons/${filename}.js`);

test(`${icon.title} has a "title"`, () => {
Expand Down
25 changes: 17 additions & 8 deletions tests/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ const simpleIcons = require('../index.js');
const { titleToFilename } = require("../scripts/utils.js");

icons.forEach(icon => {
const subject = simpleIcons[icon.title];
const name = icon.slug || icon.title;
const subject = simpleIcons[name];

test(`${icon.title} has a "title"`, () => {
expect(typeof subject.title).toBe('string');
Expand Down Expand Up @@ -31,17 +32,25 @@ icons.forEach(icon => {
expect(typeof subject.slug).toBe('string');
});

test(`${icon.title} can be found by it's title`, () => {
const found = simpleIcons.get(icon.title);
expect(found).toBeDefined();
expect(found.title).toEqual(icon.title);
});
// NOTE: Icons with custom slugs have a custom slug because their title is
// already taken, so they should explicitly not be findable by their title.
if (icon.slug === undefined) {
test(`${icon.title} can be found by it's title`, () => {
const found = simpleIcons.get(icon.title);
expect(found).toBeDefined();
expect(found.title).toEqual(icon.title);
expect(found.hex).toEqual(icon.hex);
expect(found.source).toEqual(icon.source);
});
}

test(`${icon.title} can be found by it's slug`, () => {
const name = titleToFilename(icon.title);
const found = simpleIcons.get(name);
const slug = icon.slug || titleToFilename(icon.title);
const found = simpleIcons.get(slug);
expect(found).toBeDefined();
expect(found.title).toEqual(icon.title);
expect(found.hex).toEqual(icon.hex);
expect(found.source).toEqual(icon.source);
});
});

Expand Down