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

Remove ReactDataGrid dependency from addons bundle #1272

Merged
merged 8 commits into from
Sep 11, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 4 additions & 1 deletion config/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ module.exports = function (config) {
loaders: webpackConfig.module.loaders
},
resolve: {
extensions: ['', '.webpack.js', '.web.js', '.js', '.jsx']
extensions: ['', '.webpack.js', '.web.js', '.js', '.jsx'],
alias: {
common: path.resolve('packages/common/')
}
},
plugins: [
new RewirePlugin()
Expand Down
6 changes: 6 additions & 0 deletions config/webpack.common.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const webpack = require('webpack');
const argv = require('minimist')(process.argv.slice(2));
const RELEASE = argv.release;
const path = require('path');

function getPlugins() {
const nodeEnv = RELEASE ? '"production"' : '"development"';
Expand Down Expand Up @@ -45,6 +46,11 @@ const config = {
]
},
plugins: getPlugins(),
resolve: {
alias: {
common: path.resolve('packages/common/')
}
},
postLoaders: [
{
test: /\.js$/,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const React = require('react');
const ExcelColumn = require('../../PropTypeShapes/ExcelColumn');
const ExcelColumn = require('common/prop-shapes/ExcelColumn');
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets change the require statement to ES6 import

import PropTypes from 'prop-types';

class FilterableHeaderCell extends React.Component {
Expand Down
29 changes: 29 additions & 0 deletions packages/common/constants/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as CellNavigationMode from './CellNavigationMode';
import * as EventTypes from './EventTypes';
import keyMirror from 'keymirror';


const UpdateActions = keyMirror({
CELL_UPDATE: null,
COLUMN_FILL: null,
COPY_PASTE: null,
CELL_DRAG: null
});

const DragItemTypes = {
Column: 'column'
};

const CellExpand = {
DOWN_TRIANGLE: String.fromCharCode('9660'),
Copy link
Contributor

Choose a reason for hiding this comment

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

String.fromCharCode takes a number

DOWN_TRIANGLE: String.fromCharCode(9660),

RIGHT_TRIANGLE: String.fromCharCode('9654')
};


export {
CellNavigationMode,
EventTypes,
UpdateActions,
CellExpand,
DragItemTypes
};
6 changes: 6 additions & 0 deletions packages/common/constants/zIndexes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default {
CELL_MASK: 5,
EDITOR_CONTAINER: 10,
FROZEN_CELL_MASK: 15,
FROZEN_EDITOR_CONTAINER: 20
};
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use individual exports for better error checking, treeshaking
https://medium.com/@rauschma/note-that-default-exporting-objects-is-usually-an-anti-pattern-if-you-want-to-export-the-cf674423ac38

export const CELL_MASK = 5;
..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would agree but it is a very small file, and I think it is clearer to reference zIndexes.CELL_MASK rather than just CELL_MASK

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const React = require('react');
import PropTypes from 'prop-types';
require('../../../../themes/react-data-grid-checkbox.css');
require('../../../themes/react-data-grid-checkbox.css');

class CheckboxEditor extends React.Component {
static propTypes = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const React = require('react');
const ReactDOM = require('react-dom');
const ExcelColumn = require('../PropTypeShapes/ExcelColumn');
import ExcelColumn from 'common/prop-shapes/ExcelColumn';
import PropTypes from 'prop-types';

class EditorBase extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import React from 'react';
import PropTypes from 'prop-types';
import joinClasses from 'classnames';
import SimpleTextEditor from './SimpleTextEditor';
import isFunction from'../utils/isFunction';
import { isKeyPrintable, isCtrlKeyHeldDown } from '../utils/keyboardUtils';
import zIndexes from '../constants/zIndexes';
import columnUtils from '../ColumnUtils';
require('../../../../themes/react-data-grid-core.css');
import isFunction from 'common/utils/isFunction';
import { isKeyPrintable, isCtrlKeyHeldDown } from 'common/utils/keyboardUtils';
import zIndexes from 'common/constants/zIndexes';
require('../../../themes/react-data-grid-core.css');

const isFrozen = column => column.frozen === true || column.locked === true;

class EditorContainer extends React.Component {
static displayName = 'EditorContainer';
Expand Down Expand Up @@ -322,8 +323,9 @@ class EditorContainer extends React.Component {

render() {
const { left, top, width, height, column, scrollLeft } = this.props;
const editorLeft = columnUtils.isFrozen(column) ? left + scrollLeft : left;
const style = { position: 'absolute', height, width, zIndex: zIndexes.EDITOR_CONTAINER, transform: `translate(${editorLeft}px, ${top}px)` };
const editorLeft = isFrozen(column) ? left + scrollLeft : left;
const zIndex = isFrozen(column) ? zIndexes.FROZEN_EDITOR_CONTAINER : zIndexes.EDITOR_CONTAINER;
const style = { position: 'absolute', height, width, zIndex, transform: `translate(${editorLeft}px, ${top}px)` };
return (
<div style={style} className={this.getContainerClass()} onBlur={this.handleBlur} onKeyDown={this.onKeyDown} onContextMenu={this.handleRightClick}>
{this.createEditor()}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
import ColumnMetrics from './ColumnMetrics';

export const shouldRowUpdate = (nextProps, currentProps) => {
return !(ColumnMetrics.sameColumns(currentProps.columns, nextProps.columns, ColumnMetrics.sameColumn)) ||
nextProps.row !== currentProps.row ||
currentProps.colOverscanStartIdx !== nextProps.colOverscanStartIdx ||
currentProps.colOverscanEndIdx !== nextProps.colOverscanEndIdx ||
currentProps.colVisibleStartIdx !== nextProps.colVisibleStartIdx ||
currentProps.colVisibleEndIdx !== nextProps.colVisibleEndIdx ||
currentProps.isSelected !== nextProps.isSelected ||
currentProps.isScrolling !== nextProps.isScrolling ||
nextProps.height !== currentProps.height ||
currentProps.isOver !== nextProps.isOver ||
currentProps.expandedRows !== nextProps.expandedRows ||
currentProps.canDrop !== nextProps.canDrop ||
currentProps.forceUpdate === true ||
currentProps.extraClasses !== nextProps.extraClasses;
};

export default shouldRowUpdate;
export const shouldRowUpdate = (nextProps, currentProps) => {
return currentProps.columns !== nextProps.columns ||
nextProps.row !== currentProps.row ||
currentProps.colOverscanStartIdx !== nextProps.colOverscanStartIdx ||
currentProps.colOverscanEndIdx !== nextProps.colOverscanEndIdx ||
currentProps.colVisibleStartIdx !== nextProps.colVisibleStartIdx ||
currentProps.colVisibleEndIdx !== nextProps.colVisibleEndIdx ||
currentProps.isSelected !== nextProps.isSelected ||
currentProps.isScrolling !== nextProps.isScrolling ||
nextProps.height !== currentProps.height ||
currentProps.isOver !== nextProps.isOver ||
currentProps.expandedRows !== nextProps.expandedRows ||
currentProps.canDrop !== nextProps.canDrop ||
currentProps.forceUpdate === true ||
currentProps.extraClasses !== nextProps.extraClasses;
};

export default shouldRowUpdate;
Copy link
Contributor

Choose a reason for hiding this comment

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

I usually prefer the following but I am fine with either

export default function shouldRowUpdate  {
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { List } from 'immutable';
module.exports = {
isEmptyArray: require('./isEmptyArray'),
isEmptyObject: require('./isEmptyObject'),
isFunction: require('./isFunction'),
isImmutableCollection: require('./isImmutableCollection'),
getMixedTypeValueRetriever: require('./mixedTypeValueRetriever'),
isColumnsImmutable: require('./isColumnsImmutable'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module.exports = function isColumnsImmutable(columns: Array<Column>) {
export default function isColumnsImmutable(columns) {
return (typeof Immutable !== 'undefined' && (columns instanceof Immutable.List));
};
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const isEmptyArray = (obj) => {
return Array.isArray(obj) && obj.length === 0;
};

module.exports = isEmptyArray;
const isEmptyArray = (obj) => {
return Array.isArray(obj) && obj.length === 0;
};
export default isEmptyArray;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
function isEmpty(obj) {
export default function isEmpty(obj) {
return Object.keys(obj).length === 0 && obj.constructor === Object;
}

module.exports = isEmpty;

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just combine these functions in a single utils.js file or update the index file with ES6 re-exports?

Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
/* @flow */

const isFunction = function(functionToCheck: any): boolean {
export default function isFunction(functionToCheck) {
let getType = {};
return functionToCheck && getType.toString.call(functionToCheck) === '[object Function]';
};

module.exports = isFunction;
}
7 changes: 7 additions & 0 deletions packages/common/utils/isImmutableCollection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { Iterable } from 'immutable';

export default function isImmutableCollection(objToVerify) {
return Iterable.isIterable(objToVerify);
};


Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
const getMixedTypeValueRetriever = (isImmutable) => {
export default function getMixedTypeValueRetriever(isImmutable) {
let retObj = {};
const retriever = (item, key) => { return item[key]; };
const immutableRetriever = (immutable, key) => { return immutable.get(key); };

retObj.getValue = isImmutable ? immutableRetriever : retriever;

return retObj;
};

module.exports = getMixedTypeValueRetriever;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import 'react-select/dist/react-select.css';
import React from 'react';
import PropTypes from 'prop-types';
import Select from 'react-select';
import { utils, shapes } from 'react-data-grid';
const { isEmptyArray } = utils;
const { ExcelColumn } = shapes;
import isEmptyArray from 'common/utils/isEmptyArray';
import ExcelColumn from 'common/prop-shapes/ExcelColumn';

class AutoCompleteFilter extends React.Component {
constructor(props) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { shapes } from 'react-data-grid';
const { ExcelColumn } = shapes;
import ExcelColumn from 'common/prop-shapes/ExcelColumn';

const RuleType = {
Number: 1,
Range: 2,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-data-grid-addons/src/data/RowFilterer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { utils } from 'react-data-grid';
const { getMixedTypeValueRetriever, isImmutableCollection } = utils;
import isImmutableCollection from 'common/utils/isImmutableCollection';
import getMixedTypeValueRetriever from 'common/utils/mixedTypeValueRetriever';

const filterRows = (filters, rows = []) => {
return rows.filter(r => {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-data-grid-addons/src/data/RowGrouper.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { utils } from 'react-data-grid';
import isImmutableCollection from 'common/utils/isImmutableCollection';
import Resolver from './RowGrouperResolver';
const { isImmutableCollection } = utils;


class RowGrouper {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {List} from 'immutable';
import groupBy from 'lodash/groupBy';
import { utils } from 'react-data-grid';
const { getMixedTypeValueRetriever, isImmutableMap } = utils;

import getMixedTypeValueRetriever from 'common/utils/mixedTypeValueRetriever';
import isImmutableMap from 'common/utils/isImmutableMap'
;
export default class RowGrouperResolver {

constructor(isImmutable) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-data-grid-addons/src/data/RowSorter.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { utils } from 'react-data-grid';
const { getMixedTypeValueRetriever, isImmutableCollection } = utils;
import getMixedTypeValueRetriever from 'common/utils/mixedTypeValueRetriever';
import isImmutableCollection from 'common/utils/isImmutableCollection';

const comparer = (a, b) => {
if (a > b) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-data-grid-addons/src/data/Selectors.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createSelector } from 'reselect';
import { utils } from 'react-data-grid';
const { isEmptyArray, isEmptyObject } = utils;
import isEmptyArray from 'common/utils/isEmptyArray';
import isEmptyObject from 'common/utils/isEmptyObject';
const groupRows = require('./RowGrouper');
const filterRows = require('./RowFilterer');
const sortRows = require('./RowSorter');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import { DragSource, DropTarget } from 'react-dnd';
import { HeaderCell } from 'react-data-grid';

class DraggableHeaderCell extends React.Component {
render() {
Expand All @@ -26,7 +25,7 @@ class DraggableHeaderCell extends React.Component {
style={{ width: 0, cursor: 'move', opacity }}
className={isOver && canDrop ? 'rdg-can-drop' : ''}
>
<HeaderCell {...this.props} />
{this.props.renderHeaderCell(this.props)}
</div>
)
);
Expand Down Expand Up @@ -84,7 +83,8 @@ DraggableHeaderCell.propTypes = {
connectDropTarget: PropTypes.func.isRequired,
isDragging: PropTypes.bool.isRequired,
isOver: PropTypes.bool,
canDrop: PropTypes.bool
canDrop: PropTypes.bool,
renderHeaderCell: PropTypes.fund
Copy link
Contributor

Choose a reason for hiding this comment

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

typo PropTypes.func, also it seems like it is a required prop

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be fixed

};

DraggableHeaderCell = DropTarget('Column', target, targetCollect)(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ import { shallow } from 'enzyme';

import DraggableContainer from '../DraggableContainer';
import DraggableHeaderCell from '../DraggableHeaderCell';
import { HeaderCell } from 'react-data-grid';

const HeaderCell = () => <div/>;

describe('<DraggableHeaderCell />', () => {
it('should render grid HeaderCell wrapper with cursor: move ', () => {
const wrapper = shallow(
<DraggableContainer>
<DraggableHeaderCell />
<DraggableHeaderCell renderHeaderCell={() => HeaderCell}/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? Should it be () => <HeaderCell /> instead?

</DraggableContainer>
);
expect(wrapper.find(HeaderCell));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import React, {Component} from 'react';
import html5DragDropContext from '../shared/html5DragDropContext';
import DraggableHeaderCell from './DraggableHeaderCell';
import RowDragLayer from './RowDragLayer';
import { utils } from 'react-data-grid';
const { isColumnsImmutable } = utils;
import isColumnsImmutable from 'common/utils/isColumnsImmutable';
import PropTypes from 'prop-types';

class DraggableContainer extends Component {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { DragSource } from 'react-dnd';
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import { _constants, HeaderCell } from 'react-data-grid';
const { DragItemTypes } = _constants;
import { DragItemTypes } from 'common/constants';

class DraggableHeaderCell extends Component {

Expand All @@ -26,14 +25,15 @@ class DraggableHeaderCell extends Component {
if (isDragging) {
return null;
}
return connectDragSource(<div style={{cursor: 'move'}}><HeaderCell {...this.props}/></div>);
return connectDragSource(<div style={{cursor: 'move'}}>{this.props.renderHeaderCell(this.props)}</div>);
}
}

DraggableHeaderCell.propTypes = {
connectDragSource: PropTypes.func.isRequired,
connectDragPreview: PropTypes.func.isRequired,
isDragging: PropTypes.bool.isRequired
isDragging: PropTypes.bool.isRequired,
renderHeaderCell: PropTypes.func
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, should it be a required prop? Or we can add a defaultProp

};

function collect(connect, monitor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import '../../../../themes/react-data-grid-drop-target.css';
import React from 'react';
import ReactDOM from 'react-dom';
import { DropTarget } from 'react-dnd';
import { RowComparer as shouldRowUpdate } from 'react-data-grid';
import rowComparer from 'common/utils/RowComparer';

let rowDropTarget = (Row) => class extends React.Component {

shouldComponentUpdate(nextProps) {
return shouldRowUpdate(nextProps, this.props);
return rowComparer(nextProps, this.props);
}

moveRow() {
Expand Down Expand Up @@ -51,4 +51,4 @@ function collect(connect, monitor) {
};
}

export default (Row) => DropTarget('Row', target, collect, {arePropsEqual: (nextProps, currentProps) => !shouldRowUpdate(nextProps, currentProps)})(rowDropTarget(Row));
export default (Row) => DropTarget('Row', target, collect, {arePropsEqual: (nextProps, currentProps) => !rowComparer(nextProps, currentProps)})(rowDropTarget(Row));
Loading