feat(PPDSC-1958): stop exporting emotion icons #281

mutebg · 2022-07-14T12:00:54Z

PPDSC-1958

What

Background - newskit will not want to maintain list the list of icons
What did you do: stopped exporting all emotion icons, so we will have only the icons which are used internally;
- updated all scripts and documentation
- stoped exporting icons from newskit package, we export only helper functions
- wrote a codemod to transform newskit to emotion icons
- used the codemod to transform our docs website so it does not depend on newskit icons anymore
What does the reviewers should expect

I have done:

Written unit tests against changes
Written functional tests against the component and/or NewsKit site
Updated relevant documentation

I have tested manually:

The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
The screen reader reads and flows through the elements as expected.
There are no new errors in the browser console coming from this PR.
When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

newskit-engineering · 2022-07-18T07:54:32Z

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-1958-stop-exporting-emotion-icons
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-1958-stop-exporting-emotion-icons/storybook

mutebg · 2022-07-21T08:21:31Z

src/index.ts

+export {
+  /* istanbul ignore next */
+  Svg,
+  /* istanbul ignore next */
+  toNewsKitIcon,
+  /* istanbul ignore next */
+  customToNewsKitIcon,
+  /* istanbul ignore next */
+  IndeterminateProgressIndicator,
+} from './icons';


Any ideas on how to fix this Istanbul coverage?

Xin00163

Might need to update http://localhost:8081/theme/foundation/iconography/#library as well

Xin00163 · 2022-07-25T08:40:22Z

lib/codemod/README.md

+```
+
+```sh
+npx @newskit/codemod emotion-icons <path>


Could we give some hints about the path?

Xin00163 · 2022-07-25T10:01:32Z

lib/codemod/src/transforms/__tests__/emotion-icons/emotion-icons.test.js

+const transform = require('../../emotion-icons');
+const {readFile} = require('../utils');
+
+function read(fileName) {


is it worth moving this to utils?

its worth, but not possible, moving it to another file __dirname will return different value

Xin00163 · 2022-07-25T11:24:12Z

lib/codemod/README.md

+
+```diff
+- import { IconFilledAccountTree } from 'newskit';
+ import {AccountTree as AccountTreeFilled} from '@emotion-icons/material/AccountTree';


Icon has the convention of Icon {Set} {Name}. To make it consistent, shall we follow {set} {name} for renaming, so it will be FilledAccountTree?

Xin00163 · 2022-07-25T11:30:26Z

site/pages/components/icons.mdx

+<Code>
+  {`
+    import {toNewsKitIcon} from 'newskit';
+    import {AccountTree} from '@emotion-icons/material/AccountTree';


It's AccountTreeFilled in the other example. If we were to follow the convention, it should be FilledAccountTree

Xin00163 · 2022-07-25T12:13:16Z

site/pages/components/icons.mdx

-  IconFilledYoutube,
-  IconFilledFigma,
-} from 'newskit';
+import {IconFilledFacebook} from '../../components/icons/icon-filled-facebook.tsx';


Why are we importing those?

mstuartf · 2022-07-26T10:28:40Z

lib/codemod/README.md

+```diff
+- import { IconFilledAccountTree } from 'newskit';
+ import {AccountTree as FilledAccountTree} from '@emotion-icons/material/AccountTree';
+ const IconFilledAccountTree = toNewsKitIcon(FilledAccountTree);


Also need an import for toNewsKitIcon?

mstuartf · 2022-07-26T10:54:56Z

lib/codemod/README.md

@@ -26,3 +26,25 @@ Link this package in your global `node_modules` by running:
 cd lib/codemod
 yarn link


We need a yarn step here

mstuartf · 2022-07-26T10:55:41Z

lib/codemod/README.md

@@ -26,3 +26,25 @@ Link this package in your global `node_modules` by running:
 cd lib/codemod
 yarn link
 ```


Add the run command for a local codemod: newskit-codemod [codemod] [paths]

* feat(PPDSC-1958): update emotion icons * feat(PPDSC-1958): update docs * feat(PPDSC-1958): do not export icons * feat(PPDSC-1958): update tests * feat(PPDSC-1958): add codemode for emotion-icons * feat(PPDSC-1958): add icons * feat(PPDSC-1958): fix tests * feat(PPDSC-1958): update snapshots * feat(PPDSC-1958): fix unit tests * feat(PPDSC-1958): linting * feat(PPDSC-1958): updatae codemod readme * feat(PPDSC-1958): update docs * feat(PPDSC-1958): fix stories * feat(PPDSC-1958): fix typo * feat(PPDSC-1958): clean * feat(PPDSC-1958): adjust codemod * feat(PPDSC-1958): fix icons names * feat(PPDSC-1958): cleanup * feat(PPDSC-1958): jest test * feat(PPDSC-1958): debug jest * feat(PPDSC-1958): reverse changes * feat(PPDSC-1958): update docs * feat(PPDSC-1958): update codemod readme * feat(PPDSC-1958): update readme

Stoyan Delev added 2 commits July 14, 2022 14:03

feat(PPDSC-1958): update emotion icons

c3d5ab5

feat(PPDSC-1958): update docs

ae5a93c

github-actions bot added the feature This change contains a new feature label Jul 14, 2022

feat(PPDSC-2214): fix integration tests

6976aa2

mutebg added the breaking change label Jul 18, 2022

Stoyan Delev added 5 commits July 19, 2022 12:08

feat(PPDSC-1958): do not export icons

a4f4f00

feat(PPDSC-1958): update tests

f476b90

feat(PPDSC-1958): add codemode for emotion-icons

91d3c07

feat(PPDSC-1958): add icons

0888778

feat(PPDSC-1958): fix tests

0ad33f6

mutebg added the ready for review Please assist in getting this reviewed label Jul 20, 2022

mutebg marked this pull request as ready for review July 20, 2022 07:38

mutebg requested review from jps, Xin00163, pdimova, baburay23, Vanals, evgenitsn and Srinivasan-Ramasamy as code owners July 20, 2022 07:38

Stoyan Delev added 8 commits July 20, 2022 11:22

feat(PPDSC-1958): update snapshots

6fb1d76

feat(PPDSC-1958): fix unit tests

ee953b0

feat(PPDSC-1958): linting

79cd60c

feat(PPDSC-1958): updatae codemod readme

e7283db

feat(PPDSC-1958): update docs

eaa89fe

feat(PPDSC-1958): fix stories

af7410b

feat(PPDSC-1958): fix stories

59ad42d

feat(PPDSC-1958): fix typo

c68f41e

mutebg commented Jul 21, 2022

View reviewed changes

feat(PPDSC-1958): clean

082002e

Merge branch 'main' into feat/PPDSC-1958-stop-exporting-emotion-icons

3c6c3f4

Xin00163 reviewed Jul 25, 2022

View reviewed changes

mutebg and others added 8 commits July 25, 2022 16:29

Merge branch 'main' into feat/PPDSC-1958-stop-exporting-emotion-icons

7b1777d

feat(PPDSC-1958): adjust codemod

b8d95ae

feat(PPDSC-1958): fix icons names

a3f1750

feat(PPDSC-1958): cleanup

66d9f23

feat(PPDSC-1958): jest test

8e17137

feat(PPDSC-1958): debug jest

cfb456f

feat(PPDSC-1958): reverse changes

65bb9f2

feat(PPDSC-1958): update docs

fbc28bf

mutebg requested a review from Xin00163 July 26, 2022 09:16

Merge branch 'main' into feat/PPDSC-1958-stop-exporting-emotion-icons

b0de653

mstuartf reviewed Jul 26, 2022

View reviewed changes

Stoyan Delev added 2 commits July 26, 2022 13:58

feat(PPDSC-1958): update codemod readme

f1fc494

feat(PPDSC-1958): update readme

1a526f4

Xin00163 approved these changes Jul 26, 2022

View reviewed changes

mutebg requested a review from mstuartf July 26, 2022 13:09

Srinivasan-Ramasamy approved these changes Jul 26, 2022

View reviewed changes

mutebg merged commit 38a86b4 into main Jul 26, 2022

mutebg deleted the feat/PPDSC-1958-stop-exporting-emotion-icons branch July 26, 2022 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(PPDSC-1958): stop exporting emotion icons #281

feat(PPDSC-1958): stop exporting emotion icons #281

mutebg commented Jul 14, 2022 •

edited

Loading

newskit-engineering commented Jul 18, 2022

mutebg Jul 21, 2022

Xin00163 left a comment

Xin00163 Jul 25, 2022

Xin00163 Jul 25, 2022

mutebg Jul 25, 2022

Xin00163 Jul 25, 2022

Xin00163 Jul 25, 2022

Xin00163 Jul 25, 2022

mstuartf Jul 26, 2022

mutebg Jul 26, 2022

mstuartf Jul 26, 2022

mutebg Jul 26, 2022

mstuartf Jul 26, 2022

mutebg Jul 26, 2022

		@@ -26,3 +26,25 @@ Link this package in your global `node_modules` by running:
		cd lib/codemod
		yarn link

feat(PPDSC-1958): stop exporting emotion icons #281

feat(PPDSC-1958): stop exporting emotion icons #281

Conversation

mutebg commented Jul 14, 2022 • edited Loading

newskit-engineering commented Jul 18, 2022

Choose a reason for hiding this comment

Xin00163 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mutebg commented Jul 14, 2022 •

edited

Loading