refactor(*): refactor imports, exports, and jsdocs #309

CITguy · 2018-08-11T02:01:53Z

JIRA: n/a (2018-08-10 free friday)

LGTM's

Dev LGTM
Zoom LGTM

Before

Requires importing helix-ui as a namespace to consume
HelixUI exports can be verbose to consume
elements namespace looks more like a property

import HelixUI from 'helix-ui';

const ICON_NAMES = Object.keys(HelixUI.ICONS);

class MyElement extends HelixUI.elements.HXElement { ... }

HelixUI.initialize();

After

Importing helix-ui as a namespace is optional. You could import individual items.
elements renamed to Elements to better communicate it as a namespace (breaking change)
New Utils namespace

Option 1: import module as namespace (retain status quo)

import * as HelixUI from 'helix-ui';

const ICON_NAMES = Object.keys(HelixUI.ICONS);
const KEY_MAP = Utils.KEYS;

class MyElement extends HelixUI.Elements.HXElement { ... }

HelixUI.initialize();

Option 2: import module items separately

import { initialize, ICONS, Elements, Utils } from 'helix-ui';

initialize();

const ICON_NAMES = Object.keys(ICONS);
const KEY_MAP = Utils.KEYS;

class MyElement extends Elements.HXElement { ... }

HelixUI · 2018-08-11T02:01:56Z

Deploy preview for helix-ui ready!

Built with commit cdcfb4e

https://deploy-preview-309--helix-ui.netlify.com

CITguy · 2018-08-11T02:03:03Z

rollup.config.js

@@ -38,70 +38,45 @@ let eslintPlugin = eslint({
    ],
 });

-// Intro/Outro placed INSIDE the applied dependency function
-let intro = `window.addEventListener('WebComponentsReady', function () {`;


Removed. Redundant to HelixUI.initialize() logic.

CITguy · 2018-08-11T02:04:45Z

rollup.config.js

    {
-        input: 'src/node-entry.js',


After refactoring, node-entry.js was providing nothing of value.

CITguy · 2018-08-11T02:05:41Z

src/COMPATIBILITY.md

@@ -1,158 +0,0 @@
-# Compatibility Notes


This file is no longer needed. More concise documentation was added to Guides > Polyfills in the docs.

CITguy · 2018-08-11T02:10:43Z

src/helix-ui/index.js

-    minor,
-    patch,
-};
-VERSION.toString = () => { return SEMVER }; // eslint-disable-line


🚨BREAKING CHANGE 🚨

HelixUI.VERSION changed from the complex object (seen above) to a string literal (line 3 to the right).

CITguy · 2018-08-11T02:13:26Z

src/helix-ui/index.js

-VERSION.toString = () => { return SEMVER }; // eslint-disable-line
-
-export default {
-    elements,


🚨BREAKING CHANGE 🚨

Rename HelixUI.elements namespace to HelixUI.Elements. I doubt that anybody is using this API, but we'll be sure to announce the breaking change.

CITguy · 2018-08-11T21:20:23Z

src/helix-ui/elements/index.js

@@ -0,0 +1,28 @@
+export { HXAccordionElement } from './HXAccordionElement';


file moved from src/helix-ui/elements.js

simplified import paths from ./elements/<HXFileName>.js to ./<HXFileName>

CITguy · 2018-08-11T21:22:01Z

src/helix-ui/icons.js

@@ -1,169 +0,0 @@
-import _account from './icons/account.svg';


File moved to src/helix-ui/icons/index.js with modified contents.

CITguy · 2018-08-11T21:22:29Z

src/helix-ui/icons/index.js

@@ -0,0 +1,169 @@
+import _account from './account.svg';


simplified import paths from ./icons/<icon>.svg to ./<icon>.svg

CITguy · 2018-08-11T21:23:44Z

src/helix-ui/index.js

+import * as Elements from './elements';
+export { version as VERSION } from '../../package.json';
+export { default as ICONS } from './icons';
+export { default as Utils } from './utils';


Adds HelixUI.Utils.* to generated JavaScript.

CITguy · 2018-08-11T21:28:35Z

src/helix-ui/util.js

@@ -1,3 +0,0 @@
-/** @module */


Moved to src/helix-ui/util/index.js

@imports

* add documentation around `<hx-icon>` being an inline element * add Shadow Markup for HXIconElement * add Shadow Styles for HXIconElement * remove unnecessary @imports for hx-icon styles * remove unnecessary `fill-rule` attributes from SVG files * remove HXIconElement.icons property (see #309) Closes: * SURF-1244 (inconsistent button heights w/ icons) * SURF-1260 (multiple rendering of hx-icon) * SURF-1293 (JSX: `<hx-error>`) * SURF-1272 (JSX: Icons)

@imports

* add documentation around `<hx-icon>` being an inline element * add Shadow Markup for HXIconElement * add Shadow Styles for HXIconElement * remove unnecessary @imports for hx-icon styles * remove unnecessary `fill-rule` attributes from SVG files * remove HXIconElement.icons property (see #309) * update regression snapshots Closes: * SURF-1244 (inconsistent button heights w/ icons) * SURF-1260 (multiple rendering of hx-icon) * SURF-1293 (JSX: `<hx-error>`) * SURF-1272 (JSX: Icons)

@imports

* add documentation around `<hx-icon>` being an inline element * add Shadow Markup for HXIconElement * add Shadow Styles for HXIconElement * remove unnecessary @imports for hx-icon styles * remove unnecessary `fill-rule` attributes from SVG files * remove HXIconElement.icons property (see #309) * update regression snapshots Closes: * SURF-1244 (inconsistent button heights w/ icons) * SURF-1260 (multiple rendering of hx-icon) * SURF-1293 (JSX: `<hx-error>`) * SURF-1272 (JSX: Icons) * SURF-1114 (focus style of "X" in `<hx-search>`)

@imports

* add documentation around `<hx-icon>` being an inline element * add Shadow Markup for HXIconElement * add Shadow Styles for HXIconElement * remove unnecessary @imports for hx-icon styles * remove unnecessary `fill-rule` attributes from SVG files * remove HXIconElement.icons property (see #309) * update regression snapshots Closes: * SURF-1224 (inconsistent button heights w/ icons) * SURF-1260 (multiple rendering of hx-icon) * SURF-1293 (JSX: `<hx-error>`) * SURF-1272 (JSX: Icons) * SURF-1114 (focus style of "X" in `<hx-search>`)

@imports

* add documentation around `<hx-icon>` being an inline element * add Shadow Markup for HXIconElement * add Shadow Styles for HXIconElement * remove unnecessary @imports for hx-icon styles * remove unnecessary `fill-rule` attributes from SVG files * remove HXIconElement.icons property (see #309) * update regression snapshots Closes: * SURF-1224 (inconsistent button heights w/ icons) * SURF-1260 (multiple rendering of hx-icon) * SURF-1293 (JSX: `<hx-error>`) * SURF-1272 (JSX: Icons) * SURF-1114 (focus style of "X" in `<hx-search>`)

cathysiller · 2018-08-17T17:07:20Z

src/helix-ui/utils/index.js

+export default {
+    KEYS,
+    Position,
+};


add the onScroll as well

cathysiller · 2018-08-17T17:07:32Z

src/helix-ui/utils/index.js

+ * (because 'scroll' events do not bubble).
+ *
+ * The event is dispatched from the `document` object, instead of bubbling from
+ * the oritinal element, to avoid interfering with 'scroll' event listeners


original element

* add `HelixUI.Utils` namespace * refactor utils to better adhere to ES6 module format * remove redundant `src/COMPATIBILITY.md` documentation * remove redundant initialization logic from `helix-ui.browser[.min].js` * remove unnecessary `node-entry.js` * normalize all import paths to use the Node ES6 format (no `.js` extensions) BREAKING: * rename `HelixUI.elements` namespace to `HelixUI.Elements` * convert `HelixUI.VERSION` from an object to a string literal

cathysiller

zoom lgtm

100stacks · 2018-08-17T17:49:56Z

Dev LGTM

CITguy added the #freefriday label Aug 11, 2018

CITguy force-pushed the ff-20180810 branch 3 times, most recently from 33b91e2 to 64f7ad1 Compare August 11, 2018 21:15

CITguy commented Aug 11, 2018

View reviewed changes

CITguy force-pushed the ff-20180810 branch 2 times, most recently from 4fd106b to 5f8e2f9 Compare August 12, 2018 19:11

CITguy mentioned this pull request Aug 12, 2018

refactor(modules): reorganize file structure into modules HelixDesignSystem/helix-react#72

Closed

2 tasks

CITguy mentioned this pull request Aug 14, 2018

fix(HXIconElement): add ShadowDOM and correct issues #311

Merged

3 tasks

CITguy added the 🚨 Breaking Change label Aug 14, 2018

CITguy mentioned this pull request Aug 15, 2018

[FEATURE] Deep module imports #315

Open

CITguy added the DO NOT MERGE label Aug 15, 2018

This comment has been minimized.

Sign in to view

CITguy removed the DO NOT MERGE label Aug 17, 2018

CITguy force-pushed the ff-20180810 branch 2 times, most recently from ad2558a to 90fa31d Compare August 17, 2018 15:36

cathysiller suggested changes Aug 17, 2018

View reviewed changes

CITguy force-pushed the ff-20180810 branch from 90fa31d to cdcfb4e Compare August 17, 2018 17:35

CITguy added this to the v0.13.0 milestone Aug 17, 2018

cathysiller approved these changes Aug 17, 2018

View reviewed changes

CITguy merged commit ce1cef9 into master Aug 17, 2018

CITguy deleted the ff-20180810 branch August 17, 2018 17:50

CITguy mentioned this pull request Aug 21, 2018

fix(Box): correct bugs with Box #319

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(*): refactor imports, exports, and jsdocs #309

refactor(*): refactor imports, exports, and jsdocs #309

CITguy commented Aug 11, 2018 •

edited by 100stacks

Loading

HelixUI commented Aug 11, 2018 •

edited

Loading

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

CITguy Aug 11, 2018

This comment has been minimized.

cathysiller Aug 17, 2018

cathysiller Aug 17, 2018

cathysiller left a comment

100stacks commented Aug 17, 2018

		@@ -0,0 +1,28 @@
		export { HXAccordionElement } from './HXAccordionElement';

		@@ -1,169 +0,0 @@
		import _account from './icons/account.svg';

refactor(*): refactor imports, exports, and jsdocs #309

refactor(*): refactor imports, exports, and jsdocs #309

Conversation

CITguy commented Aug 11, 2018 • edited by 100stacks Loading

LGTM's

Before

After

HelixUI commented Aug 11, 2018 • edited Loading

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

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cathysiller left a comment

Choose a reason for hiding this comment

100stacks commented Aug 17, 2018

CITguy commented Aug 11, 2018 •

edited by 100stacks

Loading

HelixUI commented Aug 11, 2018 •

edited

Loading