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

[KED-2984] Remove Kedro-UI webfont usage #731

Merged
merged 4 commits into from
Feb 10, 2022
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
10 changes: 7 additions & 3 deletions public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@
<link rel="manifest" href="%PUBLIC_URL%/manifest.json" />
<link rel="shortcut icon" href="%PUBLIC_URL%/favicon.ico" />
<title>Kedro-Viz</title>
<link rel="preconnect" href="https://fonts.googleapis.com" />
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin />
<link
href="https://fonts.googleapis.com/css2?family=Titillium+Web:wght@400;600&display=swap"
rel="stylesheet"
/>
</head>

<body>
<noscript>
You need to enable JavaScript to run this app.
</noscript>
<noscript> You need to enable JavaScript to run this app. </noscript>
<div id="root"></div>
</body>
</html>
11 changes: 0 additions & 11 deletions src/actions/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
TOGGLE_TEXT_LABELS,
TOGGLE_THEME,
UPDATE_CHART_SIZE,
UPDATE_FONT_LOADED,
TOGGLE_CODE,
TOGGLE_MODULAR_PIPELINE_FOCUS_MODE,
changeFlag,
Expand All @@ -30,7 +29,6 @@ import {
toggleTextLabels,
toggleTheme,
updateChartSize,
updateFontLoaded,
toggleFocusMode,
} from '../actions';
import {
Expand Down Expand Up @@ -274,15 +272,6 @@ describe('actions', () => {
expect(updateChartSize(chartSize)).toEqual(expectedAction);
});

it('should create an action to update the state when the webfont has loaded', () => {
const fontLoaded = true;
const expectedAction = {
type: UPDATE_FONT_LOADED,
fontLoaded,
};
expect(updateFontLoaded(fontLoaded)).toEqual(expectedAction);
});

it('should create an action to change a flag', () => {
const expectedAction = {
type: CHANGE_FLAG,
Expand Down
2 changes: 1 addition & 1 deletion src/actions/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const layoutWorker = preventWorkerQueues(worker, layout);
* Async action to calculate graph layout in a web worker
* whiled displaying a loading spinner
* @param {Object} graphState A subset of main state
* @return {function} A promise that resolves when the calcuation is done
* @return {function} A promise that resolves when the calculation is done
*/
export function calculateGraph(graphState) {
if (!graphState) {
Expand Down
13 changes: 0 additions & 13 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,6 @@ export function updateZoom(zoom) {
};
}

export const UPDATE_FONT_LOADED = 'UPDATE_FONT_LOADED';

/**
* Update whether the webfont has loaded, which should block the chart render
* @param {Boolean} fontLoaded Whether the font has loaded
*/
export function updateFontLoaded(fontLoaded) {
return {
type: UPDATE_FONT_LOADED,
fontLoaded,
};
}

export const TOGGLE_MINIMAP = 'TOGGLE_MINIMAP';

/**
Expand Down
19 changes: 1 addition & 18 deletions src/components/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Provider } from 'react-redux';
import 'what-input';
import '@quantumblack/kedro-ui/lib/styles/app-no-webfont.css';
import LoadWebFont from '@quantumblack/kedro-ui/lib/utils/webfont.js';
import configureStore from '../../store';
import { resetData, updateFontLoaded } from '../../actions';
import { resetData } from '../../actions';
import { loadInitialPipelineData } from '../../actions/pipelines';
import Wrapper from '../wrapper';
import getInitialState, {
Expand Down Expand Up @@ -33,7 +31,6 @@ class App extends React.Component {
if (this.props.data === 'json') {
this.store.dispatch(loadInitialPipelineData());
}
this.loadWebFonts();
this.announceFlags(this.store.getState().flags);
}

Expand All @@ -54,20 +51,6 @@ class App extends React.Component {
}
}

/**
* Dispatch an action once the webfont has loaded.
* Uses https://github.com/typekit/webfontloader
*/
loadWebFonts() {
const setFontLoaded = () => {
this.store.dispatch(updateFontLoaded(true));
};
LoadWebFont({
active: setFontLoaded,
inactive: setFontLoaded,
});
}

/**
* Dispatch an action to update the store with new pipeline data
*/
Expand Down
9 changes: 9 additions & 0 deletions src/components/app/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,12 @@
--color-base-20: rgba(255, 255, 255, 0.2);
--color-black-10: rgba(0, 0, 0, 0.1);
}

.kedro {
-webkit-font-smoothing: antialiased;
font-family: 'Titillium Web', sans-serif;
font-size: 10px;
font-weight: 400;
letter-spacing: 0.1px;
line-height: 1.4;
}
10 changes: 9 additions & 1 deletion src/components/app/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { mockState } from '../../utils/state.mock';
import { Flags } from '../../utils/flags';
import { saveState } from '../../store/helpers';
import { prepareNonPipelineState } from '../../store/initial-state';
import reducer from '../../reducers/index';
import { TOGGLE_GRAPH_LOADING } from '../../actions/graph';

describe('App', () => {
const getState = (wrapper) => wrapper.instance().store.getState();
Expand Down Expand Up @@ -68,7 +70,13 @@ describe('App', () => {
test('but does not override non-pipeline values', () => {
const wrapper = shallow(<App data={demo} />);
wrapper.setProps({ data: spaceflights });
expect(getState(wrapper)).toMatchObject(prepareNonPipelineState({}));

const newState = reducer(getState(wrapper), {
type: TOGGLE_GRAPH_LOADING,
loading: false,
});

expect(newState).toMatchObject(prepareNonPipelineState({}));
});
});

Expand Down
2 changes: 0 additions & 2 deletions src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
TOGGLE_THEME,
UPDATE_CHART_SIZE,
UPDATE_ZOOM,
UPDATE_FONT_LOADED,
TOGGLE_IGNORE_LARGE_WARNING,
TOGGLE_PRETTY_NAME,
} from '../actions';
Expand Down Expand Up @@ -68,7 +67,6 @@ const combinedReducer = combineReducers({
// These props have very simple non-nested actions
chartSize: createReducer({}, UPDATE_CHART_SIZE, 'chartSize'),
zoom: createReducer({}, UPDATE_ZOOM, 'zoom'),
fontLoaded: createReducer(false, UPDATE_FONT_LOADED, 'fontLoaded'),
textLabels: createReducer(true, TOGGLE_TEXT_LABELS, 'textLabels'),
theme: createReducer('dark', TOGGLE_THEME, 'theme'),
prettyName: createReducer(true, TOGGLE_PRETTY_NAME, 'prettyName'),
Expand Down
22 changes: 11 additions & 11 deletions src/reducers/reducers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
TOGGLE_TEXT_LABELS,
TOGGLE_THEME,
UPDATE_CHART_SIZE,
UPDATE_FONT_LOADED,
} from '../actions';
import {
TOGGLE_NODE_CLICKED,
Expand All @@ -32,6 +31,7 @@ import {
} from '../actions/node-type';
import { UPDATE_ACTIVE_PIPELINE } from '../actions/pipelines';
import { TOGGLE_MODULAR_PIPELINE_ACTIVE } from '../actions/modular-pipelines';
import { TOGGLE_GRAPH_LOADING } from '../actions/graph';

describe('Reducer', () => {
it('should return an Object', () => {
Expand Down Expand Up @@ -343,16 +343,6 @@ describe('Reducer', () => {
});
});

describe('UPDATE_FONT_LOADED', () => {
it('should update the state when the webfont is loaded', () => {
const newState = reducer(mockState.spaceflights, {
type: UPDATE_FONT_LOADED,
fontLoaded: true,
});
expect(newState.fontLoaded).toBe(true);
});
});

describe('TOGGLE_CODE', () => {
it('should toggle whether the code panel is open', () => {
const newState = reducer(mockState.spaceflights, {
Expand Down Expand Up @@ -395,4 +385,14 @@ describe('Reducer', () => {
expect(newState.modularPipeline.active).toEqual({ nested: true });
});
});

describe('TOGGLE_GRAPH_LOADING', () => {
it('should toggle the loading state of the graph', () => {
const newState = reducer(mockState.spaceflights, {
type: TOGGLE_GRAPH_LOADING,
loading: true,
});
expect(newState.loading.graph).toBe(true);
});
});
});
8 changes: 3 additions & 5 deletions src/selectors/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
const getSizeWarningFlag = (state) => state.flags.sizewarning;
const getVisibleSidebar = (state) => state.visible.sidebar;
const getVisibleCode = (state) => state.visible.code;
const getFontLoaded = (state) => state.fontLoaded;
const getIgnoreLargeWarning = (state) => state.ignoreLargeWarning;
const getGraphHasNodes = (state) => Boolean(state.graph?.nodes?.length);
const getChartSizeState = (state) => state.chartSize;
Expand Down Expand Up @@ -49,15 +48,14 @@ export const getGraphInput = createSelector(
getVisibleNodes,
getVisibleEdges,
getVisibleLayerIDs,
getFontLoaded,
getTriggerLargeGraphWarning,
],
(nodes, edges, layers, fontLoaded, triggerLargeGraphWarning) => {
if (!fontLoaded || triggerLargeGraphWarning) {
(nodes, edges, layers, triggerLargeGraphWarning) => {
if (triggerLargeGraphWarning) {
return null;
}

return { nodes, edges, layers, fontLoaded };
return { nodes, edges, layers };
}
);

Expand Down
12 changes: 1 addition & 11 deletions src/selectors/layout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ import {
getGraphInput,
getTriggerLargeGraphWarning,
} from './layout';
import {
changeFlag,
toggleIgnoreLargeWarning,
updateFontLoaded,
} from '../actions';
import { changeFlag, toggleIgnoreLargeWarning } from '../actions';
import { updateGraph } from '../actions/graph';
import { toggleTypeDisabled } from '../actions/node-type';
import reducer from '../reducers';
Expand Down Expand Up @@ -113,15 +109,9 @@ describe('Selectors', () => {
nodes: expect.any(Array),
edges: expect.any(Array),
layers: expect.any(Array),
fontLoaded: expect.any(Boolean),
})
);
});

it('returns null if fontLoaded is false', () => {
const newMockState = reducer(mockState, updateFontLoaded(false));
expect(getGraphInput(newMockState)).toEqual(null);
});
});

describe('getSidebarWidth', () => {
Expand Down
7 changes: 3 additions & 4 deletions src/selectors/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createSelector } from 'reselect';

const getGraphLoading = (state) => state.loading.graph;
const getPipelineLoading = (state) => state.loading.pipeline;
const getFontLoading = (state) => !state.fontLoaded;
const getNodeLoading = (state) => state.loading.node;

export const getDisplayLargeGraph = (state) => state.ignoreLargeWarning;
Expand All @@ -11,8 +10,8 @@ export const getDisplayLargeGraph = (state) => state.ignoreLargeWarning;
* Determine whether to show the loading spinner
*/
export const isLoading = createSelector(
[getGraphLoading, getPipelineLoading, getFontLoading, getNodeLoading],
(graphLoading, pipelineLoading, fontLoading, nodeLoading) => {
return graphLoading || pipelineLoading || fontLoading || nodeLoading;
[getGraphLoading, getPipelineLoading, getNodeLoading],
(graphLoading, pipelineLoading, nodeLoading) => {
return graphLoading || pipelineLoading || nodeLoading;
}
);
47 changes: 15 additions & 32 deletions src/selectors/nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const getPrettyName = (state) => state.prettyName;
const getTagActive = (state) => state.tag.active;
const getModularPipelineActive = (state) => state.modularPipeline.active;
const getTextLabels = (state) => state.textLabels;
const getFontLoaded = (state) => state.fontLoaded;
const getNodeTypeDisabled = (state) => state.nodeType.disabled;
const getClickedNode = (state) => state.node.clicked;
const getEdgeIDs = (state) => state.edge.ids;
Expand Down Expand Up @@ -245,11 +244,8 @@ export const getGroupedNodes = createSelector([getNodeData], (nodes) =>
* measure its width with getBBox, then delete the container and store the value
*/
export const getNodeTextWidth = createSelector(
[getPipelineNodeIDs, getNodeLabel, getFontLoaded],
(nodeIDs, nodeLabel, fontLoaded) => {
if (!fontLoaded) {
return {};
}
[getPipelineNodeIDs, getNodeLabel],
(nodeIDs, nodeLabel) => {
const nodeTextWidth = {};
const svg = select(document.body)
.append('svg')
Expand Down Expand Up @@ -299,17 +295,8 @@ export const getPadding = (showLabels, nodeType) => {
* Calculate node width/height and icon/text positioning
*/
export const getNodeSize = createSelector(
[
getPipelineNodeIDs,
getNodeTextWidth,
getTextLabels,
getNodeType,
getFontLoaded,
],
(nodeIDs, nodeTextWidth, textLabels, nodeType, fontLoaded) => {
if (!fontLoaded) {
return {};
}
[getPipelineNodeIDs, getNodeTextWidth, getTextLabels, getNodeType],
(nodeIDs, nodeTextWidth, textLabels, nodeType) => {
return arrayToObject(nodeIDs, (nodeID) => {
const iconSize = textLabels ? 24 : 28;
const padding = getPadding(textLabels, nodeType[nodeID]);
Expand Down Expand Up @@ -342,7 +329,6 @@ export const getVisibleNodes = createSelector(
getNodeSize,
getNodeLayer,
getNodeRank,
getFontLoaded,
],
(
nodeIDs,
Expand All @@ -352,21 +338,18 @@ export const getVisibleNodes = createSelector(
nodeFullName,
nodeSize,
nodeLayer,
nodeRank,
fontLoaded
nodeRank
) =>
fontLoaded
? nodeIDs.map((id) => ({
id,
name: nodeLabel[id],
fullName: nodeFullName[id],
icon: getShortType([nodeDatasetType[id]], nodeType[id]),
type: nodeType[id],
layer: nodeLayer[id],
rank: nodeRank[id],
...nodeSize[id],
}))
: []
nodeIDs.map((id) => ({
id,
name: nodeLabel[id],
fullName: nodeFullName[id],
icon: getShortType([nodeDatasetType[id]], nodeType[id]),
type: nodeType[id],
layer: nodeLayer[id],
rank: nodeRank[id],
...nodeSize[id],
}))
);

/**
Expand Down
Loading