Skip to content

Commit

Permalink
refactor(icon): update dialtone icons usage (#1295)
Browse files Browse the repository at this point in the history
* refactor: update dialtone icons usage

* update dialtone-icons

* prevent duplicated keys

* fix tests

* change to computed prop

* fix tests

* make dialtone-icons external

* move icons to devDependencies and add to peerDependencies

* update package-lock.json

* externalize all node_modules deps

* disable duplicate-id a11y rule

* fix icons visual tests

* update percy cli

* disable cache on test
  • Loading branch information
juliodialpad authored Nov 8, 2023
1 parent 6695eb6 commit ab0ce7d
Show file tree
Hide file tree
Showing 25 changed files with 841 additions and 300 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/visual_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@ jobs:
env:
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }}
PERCY_TARGET_BRANCH: ${{ github.base_ref }}
run: npx percy storybook --config percy.config.cjs --verbose ./storybook-static
run: npx percy storybook --disable-cache --config percy.config.cjs --verbose ./storybook-static
14 changes: 10 additions & 4 deletions .storybook/preview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ export default {
a11y: {
config: {
// This is a legitimate color contrast issue that needs to be fixed by the design team in the future.
rules: [{
id: 'color-contrast',
reviewOnFail: true,
}],
rules: [
{
id: 'color-contrast',
reviewOnFail: true,
},
{
id: 'duplicate-id',
enabled: false,
}
],
},
},
controls: {
Expand Down
5 changes: 2 additions & 3 deletions common/storybook_utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as dialtoneIcons from '@dialpad/dialtone-icons';
import { pascalCaseToKebabCase } from '@/common/utils';
import iconNames from '@dialpad/dialtone-icons/dist/icons.json';

/**
* Will use a Vue SFC to render the template rather than a template string.
Expand All @@ -25,7 +24,7 @@ export const createTemplateFromVueFile = (args, argTypes, templateComponent) =>
* @returns {string[]} icon component names
*/
export function getIconNames () {
return [undefined, ...Object.keys(dialtoneIcons).map(name => pascalCaseToKebabCase(name))];
return [undefined, ...iconNames];
}

export const generateTemplate = (component,
Expand Down
1 change: 1 addition & 0 deletions components/avatar/avatar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe('DtAvatar Tests', () => {
mockProps = { iconName: 'accessibility' };

updateWrapper();
await vi.dynamicImportSettled();
});

it('icon should exist', () => {
Expand Down
1 change: 1 addition & 0 deletions components/badge/badge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ describe('DtBadge Tests', () => {
mockProps = { type: 'ai' };

updateWrapper();
await vi.dynamicImportSettled();
});

it('should have correct type', async () => {
Expand Down
10 changes: 7 additions & 3 deletions components/datepicker/datepicker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,23 @@ describe('DtDatepicker Tests', () => {
let nextMonthButton;
let nextYearButton;

const updateWrapper = () => {
const updateWrapper = async () => {
wrapper = mount(DtDatepicker, {
propsData: { ...baseProps, ...mockProps },
attachTo: document.body,
});

await vi.dynamicImportSettled();

datepickerHeader = wrapper.find('.d-datepicker--header');
prevYearButton = wrapper.find('#prevYearButton');
prevMonthButton = wrapper.find('#prevMonthButton');
nextMonthButton = wrapper.find('#nextMonthButton');
nextYearButton = wrapper.find('#nextYearButton');
};

beforeEach(() => {
updateWrapper();
beforeEach(async () => {
await updateWrapper();
});

afterEach(() => {
Expand Down Expand Up @@ -131,11 +133,13 @@ describe('DtDatepicker Tests', () => {

it('previous month button should has correct aria label', () => {
expect(prevMonthButton.attributes('aria-label'))
// eslint-disable-next-line max-len
.toContain(`${baseProps.changeToLabel} ${baseProps.prevMonthLabel} ${formatMonth(MOCK_TODAY_MONTH - 1, MONTH_FORMAT)}`);
});

it('next month button should has correct aria label', () => {
expect(nextMonthButton.attributes('aria-label'))
// eslint-disable-next-line max-len
.toContain(`${baseProps.changeToLabel} ${baseProps.nextMonthLabel} ${formatMonth(MOCK_TODAY_MONTH + 1, MONTH_FORMAT)}`);
});

Expand Down
12 changes: 10 additions & 2 deletions components/icon/icon.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ export const Default = {

export const Variants = {
render: VariantsTemplate,
args: {},
parameters: { options: { showPanel: false }, controls: { disable: true } },
args: { limit: undefined },
parameters: {
percy: {
args: {
limit: 10,
},
},
options: { showPanel: false },
controls: { disable: true },
},
};
15 changes: 8 additions & 7 deletions components/icon/icon.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ let mockProps = {};
describe('DtIcon Tests', () => {
let wrapper;

const updateWrapper = () => {
const updateWrapper = async () => {
wrapper = mount(DtIcon, {
props: { ...baseProps, ...mockProps },
});
await vi.dynamicImportSettled();
};

beforeEach(() => {
updateWrapper();
beforeEach(async () => {
await updateWrapper();
});

afterEach(() => {
Expand All @@ -35,10 +36,10 @@ describe('DtIcon Tests', () => {
});

describe('When size prop is set', () => {
it('Should have correct class', () => {
it('Should have correct class', async () => {
mockProps = { size: '800' };

updateWrapper();
await updateWrapper();

expect(wrapper.classes().includes('d-icon--size-800')).toBe(true);
});
Expand All @@ -47,10 +48,10 @@ describe('DtIcon Tests', () => {

describe('Accessibility Tests', () => {
describe('When ariaLabel prop is set', () => {
beforeEach(() => {
beforeEach(async () => {
mockProps = { ariaLabel: 'icon description' };

updateWrapper();
await updateWrapper();
});

it('sets the aria-label attribute', () => {
Expand Down
31 changes: 21 additions & 10 deletions components/icon/icon.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<template>
<component
:is="currentIcon"
v-if="currentIcon"
:is="icon"
v-if="icon"
:id="id"
data-qa="dt-icon"
:aria-hidden="ariaLabel ? 'false' : 'true'"
:aria-label="ariaLabel"
Expand All @@ -10,8 +11,9 @@
</template>

<script>
import { ICON_SIZE_MODIFIERS, DIALTONE_ICONS } from './icon_constants';
import { kebabCaseToPascalCase } from '@/common/utils';
import { ICON_SIZE_MODIFIERS } from './icon_constants';
import { defineAsyncComponent } from 'vue';
import { getUniqueString } from '@/common/utils.js';
/**
* The Icon component provides a set of glyphs and sizes to provide context your application.
Expand All @@ -21,6 +23,16 @@ export default {
name: 'DtIcon',
props: {
/**
* DtIcon identifier
*/
id: {
type: String,
default () {
return getUniqueString();
},
},
/**
* The size of the icon.
* @values 100, 200, 300, 400, 500, 600, 700, 800
Expand Down Expand Up @@ -53,12 +65,11 @@ export default {
return ICON_SIZE_MODIFIERS[this.size];
},
iconName () {
return kebabCaseToPascalCase(this.name);
},
currentIcon () {
return DIALTONE_ICONS[this.iconName];
icon () {
const name = this.name;
return defineAsyncComponent(() =>
import(`../../node_modules/@dialpad/dialtone-icons/dist/svg/${name}.svg`),
);
},
},
};
Expand Down
3 changes: 0 additions & 3 deletions components/icon/icon_constants.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as dialtoneIcons from '@dialpad/dialtone-icons';
export const ICON_SIZE_MODIFIERS = {
100: 'd-icon--size-100',
200: 'd-icon--size-200',
Expand All @@ -9,9 +8,7 @@ export const ICON_SIZE_MODIFIERS = {
700: 'd-icon--size-700',
800: 'd-icon--size-800',
};
export const DIALTONE_ICONS = dialtoneIcons;

export default {
ICON_SIZE_MODIFIERS,
DIALTONE_ICONS,
};
26 changes: 19 additions & 7 deletions components/icon/icon_variants.story.vue
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
<template>
<div class="d-flow4">
<dt-icon
v-for="size in sizes"
:key="size"
:size="size"
name="alert-circle"
/>
<div>
<template
v-for="[category, icons] in Object.entries(categories)"
:key="category"
>
<h2
class="d-tt-capitalize d-my8"
v-text="category"
/>
<dt-icon
v-for="icon in Object.keys(icons).slice(0, $attrs.limit)"
:key="`${category}-${icon}`"
:name="icon"
class="d-m8"
/>
</template>
</div>
</template>

<script>
import DtIcon from './icon.vue';
import { categories } from '@dialpad/dialtone-icons/dist/keywords.json';
export default {
name: 'IconDefault',
Expand All @@ -27,6 +37,8 @@ export default {
'700',
'800',
],
categories,
};
},
};
Expand Down
12 changes: 0 additions & 12 deletions components/input/input.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,6 @@ export const WithBothIcons = {
leftIcon: 'send',
rightIcon: 'lock-filled',
},
parameters: {
a11y: {
config: {
rules: [
{
id: 'duplicate-id',
enabled: false,
},
],
},
},
},
};

export const WithWarning = {
Expand Down
1 change: 1 addition & 0 deletions components/list_item/list_item.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe('DtListItem tests', () => {
describe('When item is selected', () => {
it('should render checkmark icon', async () => {
await wrapper.setProps({ selected: true });
await vi.dynamicImportSettled();

const icon = wrapper.find('[data-qa="dt-icon"]');

Expand Down
15 changes: 8 additions & 7 deletions components/notice/notice_icon.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,24 @@ describe('DtNoticeIcon tests', () => {
let props;
let slotsData;

const _setWrappers = () => {
const _setWrappers = async () => {
wrapper = mount(DtNoticeIcon, {
global: { components: { 'dt-icon': DtIcon } },
props,
slots: slotsData,
});
_setChildWrappers();
await _setChildWrappers();
};

const _setChildWrappers = () => {
const _setChildWrappers = async () => {
await vi.dynamicImportSettled();
icon = wrapper.findComponent(DtIcon);
};

beforeEach(function () {
beforeEach(async function () {
props = baseProps;
slotsData = baseSlotsData;
_setWrappers();
await _setWrappers();
});

describe('Presentation Tests', () => {
Expand All @@ -52,7 +53,7 @@ describe('DtNoticeIcon tests', () => {
describe('When kind is base', () => {
beforeEach(async () => {
await wrapper.setProps({ kind: 'base' });
_setChildWrappers();
await _setChildWrappers();
});

it('Should render base icon', () => {
Expand All @@ -65,7 +66,7 @@ describe('DtNoticeIcon tests', () => {
slotsData = {
default: '<dt-icon name="accessibility" />',
};
_setWrappers();
await _setWrappers();
});

it('Should render correctly', async () => {
Expand Down
Loading

0 comments on commit ab0ce7d

Please sign in to comment.