Skip to content

Commit

Permalink
fix: use original tag name when slugified one is not valid (#553)
Browse files Browse the repository at this point in the history
* use original tag name when slugified one is not valid

* use wrapper function when using slugify
  • Loading branch information
woonmayer authored and RomanHotsiy committed Jul 17, 2018
1 parent 76906eb commit 8817d9c
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
5 changes: 2 additions & 3 deletions src/services/MarkdownRenderer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as marked from 'marked';
import slugify from 'slugify';

import { highlight, html2Str } from '../utils';
import { highlight, html2Str, safeSlugify } from '../utils';
import { AppStore } from './AppStore';
import { SECTION_ATTR } from './MenuStore';

Expand Down Expand Up @@ -56,7 +55,7 @@ export class MarkdownRenderer {
parentId?: string,
): MarkdownHeading {
const item = {
id: parentId ? `${parentId}/${slugify(name)}` : `section/${slugify(name)}`,
id: parentId ? `${parentId}/${safeSlugify(name)}` : `section/${safeSlugify(name)}`,
name,
items: [],
};
Expand Down
4 changes: 2 additions & 2 deletions src/services/models/Group.model.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { action, observable } from 'mobx';
import slugify from 'slugify';

import { OpenAPIExternalDocumentation, OpenAPITag } from '../../types';
import { safeSlugify } from '../../utils';
import { MarkdownHeading } from '../MarkdownRenderer';
import { ContentItemModel } from '../MenuBuilder';
import { IMenuItem, MenuItemGroupType } from '../MenuStore';
Expand Down Expand Up @@ -32,7 +32,7 @@ export class GroupModel implements IMenuItem {
parent?: GroupModel,
) {
// markdown headings already have ids calculated as they are needed for heading anchors
this.id = (tagOrGroup as MarkdownHeading).id || type + '/' + slugify(tagOrGroup.name);
this.id = (tagOrGroup as MarkdownHeading).id || type + '/' + safeSlugify(tagOrGroup.name);
this.type = type;
this.name = tagOrGroup['x-displayName'] || tagOrGroup.name;
this.description = tagOrGroup.description || '';
Expand Down
13 changes: 12 additions & 1 deletion src/utils/__tests__/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { mapWithLast, appendToMdHeading, mergeObjects } from '../helpers';
import slugify from 'slugify';
import { mapWithLast, appendToMdHeading, mergeObjects, safeSlugify } from '../helpers';

describe('Utils', () => {
describe('helpers', () => {
Expand Down Expand Up @@ -39,6 +40,16 @@ describe('Utils', () => {
expect(val).toEqual('# Authentication\n\n<test>');
});

test('slugifyIfAvailable returns original value when cannot slugify the value', () => {
const willBeSlugifed = safeSlugify('some string')
expect(willBeSlugifed).toEqual('some-string');

const cannotBeSlugified = '가나다라 마바사'
// if slugify() fixes this issue, safeSlugify should be removed and replaced with original one.
expect(slugify(cannotBeSlugified)).toEqual('');
expect(safeSlugify(cannotBeSlugified)).toEqual('가나다라-마바사');
})

describe('mergeObjects', () => {
test('should merge Objects and all nested Ones', () => {
const obj1 = { a: { a1: 'A1' }, c: 'C', d: {} };
Expand Down
17 changes: 17 additions & 0 deletions src/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import slugify from 'slugify';

/**
* Maps over array passing `isLast` bool to iterator as the second arguemnt
*/
Expand Down Expand Up @@ -116,3 +118,18 @@ const isObject = (item: any): boolean => {
const isMergebleObject = (item): boolean => {
return isObject(item) && !Array.isArray(item);
};

/**
* slugify() returns empty string when failed to slugify.
* so try to return minimun slugified-string with failed one which keeps original value
* the regex codes are referenced with https://gist.github.com/mathewbyrne/1280286
*/
export function safeSlugify(value: string): string {
return slugify(value) ||
value.toString().toLowerCase()
.replace(/\s+/g, '-') // Replace spaces with -
.replace(/&/g, '-and-') // Replace & with 'and'
.replace(/\--+/g, '-') // Replace multiple - with single -
.replace(/^-+/, '') // Trim - from start of text
.replace(/-+$/, ''); // Trim - from end of text
}

0 comments on commit 8817d9c

Please sign in to comment.