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.

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

Extract setState queuing into a service, re-use it in EuiIcon #2565

Merged
merged 2 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Added `euiTextBreakWord()` to `EuiToast` header ([#2549](https://github.com/elastic/eui/pull/2549))
- Fixed `.eui-textBreakAll` on Firefox ([#2549](https://github.com/elastic/eui/pull/2549))
- Fixed `EuiBetaBadge` accessibility with `tab-index=0` ([#2559](https://github.com/elastic/eui/pull/2559))
- Improved `EuiIcon` loading performance ([#2565](https://github.com/elastic/eui/pull/2565))

## [`16.0.1`](https://github.com/elastic/eui/tree/v16.0.1)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
class="euiContextMenu__itemLayout"
>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__icon"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__icon"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the 2 affected context menu tests/snapshots don't care what the icon state is so I updated the snapshot instead of "fixing" the test.

focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M10.843 13.069L6.232 8.384a.546.546 0 0 1 0-.768l4.61-4.685a.552.552 0 0 0 0-.771.53.53 0 0 0-.759 0l-4.61 4.684a1.65 1.65 0 0 0 0 2.312l4.61 4.684a.53.53 0 0 0 .76 0 .552.552 0 0 0 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
<span
class="euiContextMenu__text"
>
Expand All @@ -148,18 +143,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2a
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
<button
Expand All @@ -175,18 +165,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2b
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
<button
Expand All @@ -202,18 +187,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2c
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
class="euiContextMenu__itemLayout"
>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__icon"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__icon"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M10.843 13.069L6.232 8.384a.546.546 0 0 1 0-.768l4.61-4.685a.552.552 0 0 0 0-.771.53.53 0 0 0-.759 0l-4.61 4.684a1.65 1.65 0 0 0 0 2.312l4.61 4.684a.53.53 0 0 0 .76 0 .552.552 0 0 0 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
<span
class="euiContextMenu__text"
>
Expand All @@ -148,18 +143,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2a
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
<button
Expand All @@ -175,18 +165,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2b
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
<button
Expand All @@ -202,18 +187,13 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the
2c
</span>
<svg
class="euiIcon euiIcon--medium euiIcon-isLoaded euiContextMenu__arrow"
class="euiIcon euiIcon--medium euiIcon-isLoading euiContextMenu__arrow"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.157 13.069l4.611-4.685a.546.546 0 0 0 0-.768L5.158 2.93a.552.552 0 0 1 0-.771.53.53 0 0 1 .759 0l4.61 4.684c.631.641.63 1.672 0 2.312l-4.61 4.684a.53.53 0 0 1-.76 0 .552.552 0 0 1 0-.771z"
fill-rule="nonzero"
/>
</svg>
/>
</span>
</button>
</div>
Expand Down
28 changes: 5 additions & 23 deletions src/components/datagrid/data_grid_inmemory_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import React, {
useMemo,
useState,
} from 'react';
import { createPortal, unstable_batchedUpdates } from 'react-dom';
import { createPortal } from 'react-dom';
import {
EuiDataGridCellValueElementProps,
EuiDataGridCellProps,
} from './data_grid_cell';
import { EuiDataGridColumn, EuiDataGridInMemory } from './data_grid_types';
import { enqueueStateChange } from '../../services/react';

interface EuiDataGridInMemoryRendererProps {
inMemory: EuiDataGridInMemory;
Expand All @@ -27,27 +28,6 @@ interface EuiDataGridInMemoryRendererProps {

function noop() {}

const _queue: Function[] = [];

function processQueue() {
// the queued functions trigger react setStates which, if unbatched,
// each cause a full update->render->dom pass _per function_
// instead, tell React to wait until all updates are finished before re-rendering
unstable_batchedUpdates(() => {
for (let i = 0; i < _queue.length; i++) {
_queue[i]();
}
_queue.length = 0;
});
}

function enqueue(fn: Function) {
if (_queue.length === 0) {
setTimeout(processQueue);
}
_queue.push(fn);
}

function getElementText(element: HTMLElement) {
return 'innerText' in element
? element.innerText
Expand All @@ -71,7 +51,9 @@ const ObservedCell: FunctionComponent<{
onCellRender(i, column, getElementText(ref));
const observer = new MutationObserver(() => {
// onMutation callbacks aren't in the component lifecycle, intentionally batch any effects
enqueue(onCellRender.bind(null, i, column, getElementText(ref)));
enqueueStateChange(
onCellRender.bind(null, i, column, getElementText(ref))
);
});
observer.observe(ref, {
characterData: true,
Expand Down
8 changes: 4 additions & 4 deletions src/components/icon/icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ const prettyHtml = cheerio.load('');

function testIcon(props: PropsOf<EuiIcon>) {
return () => {
const component = mount(<EuiIcon {...props} />);

expect.assertions(1);
return new Promise(resolve => {
setTimeout(() => {
const onIconLoad = () => {
component.update();
expect(prettyHtml(component.html())).toMatchSnapshot();
resolve();
}, 0);
};
const component = mount(<EuiIcon {...props} onIconLoad={onIconLoad} />);
});
};
}
Expand Down
28 changes: 22 additions & 6 deletions src/components/icon/icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { CommonProps, keysOf } from '../common';
// TS file (dev/docs) or the JS file (distributed), and it's more effort than worth
// to generate & git track a TS module definition for each icon component
import { icon as empty } from './assets/empty.js';
import { enqueueStateChange } from '../../services/react';

const typeToPathMap = {
addDataApp: 'app_add_data',
Expand Down Expand Up @@ -428,6 +429,10 @@ export type EuiIconProps = CommonProps &
* Note that every size other than `original` assumes the provided SVG sits on a square viewbox.
*/
size?: IconSize;
/**
* Callback when the icon has been loaded & rendered
*/
onIconLoad?: () => void;
};

interface State {
Expand Down Expand Up @@ -500,12 +505,22 @@ export class EuiIcon extends PureComponent<EuiIconProps, State> {
// eslint-disable-next-line prefer-template
'./assets/' + typeToPathMap[iconType] + '.js'
).then(({ icon }) => {
if (this.isMounted) {
this.setState({
icon,
isLoading: false,
});
}
enqueueStateChange(() => {
if (this.isMounted) {
this.setState(
{
icon,
isLoading: false,
},
() => {
const { onIconLoad } = this.props;
if (onIconLoad) {
onIconLoad();
}
}
);
}
});
});
};

Expand All @@ -516,6 +531,7 @@ export class EuiIcon extends PureComponent<EuiIconProps, State> {
color,
className,
tabIndex,
onIconLoad,
...rest
} = this.props;

Expand Down
22 changes: 22 additions & 0 deletions src/services/react.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { unstable_batchedUpdates } from 'react-dom';

const _queue: Function[] = [];

function processQueue() {
// the queued functions trigger react setStates which, if unbatched,
// each cause a full update->render->dom pass _per function_
// instead, tell React to wait until all updates are finished before re-rendering
unstable_batchedUpdates(() => {
for (let i = 0; i < _queue.length; i++) {
_queue[i]();
}
_queue.length = 0;
});
}

export function enqueueStateChange(fn: Function) {
if (_queue.length === 0) {
setTimeout(processQueue);
}
_queue.push(fn);
}